[shared_preferences] Documented return values for remove and set methods.

[RamonFarizel]

This PR adds documentation about the remove and set methods.
Those methods are basically following the get methods patterns, they throw an exception when the type is not expected.
Also, returns true when the operation succeeded.

List which issues are fixed by this PR. You must list at least one issue.
fixes: #146070

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[tarrinneal]

All of the changelogs and pubspecs changes for the sub packages (android, foundation, etc) should be reverted. Only the subpackages with changes need them.

[tarrinneal]

I'm of the opinion that these changes to the doc comments aren't necessary, as the methods are self documenting.

You can't call setString with anything that isn't a string. Same with the other set methods.

The "returns true" comments are unfortunately not exactly correct, as some platforms don't actually have any response that verifies success. If I were to design this api myself today, I wouldn't have any bool returns for that reason.

@stuartmorgan what do you think?

[stuartmorgan-g]

as the methods are self documenting

I don't think the return value is self-documenting; I literally had to go back and read every platform's implementation to figure out what it actually does, and I wrote several of those implementations.

I think it's worth trying to describe the reality of what they do as best we can, which I agree the proposed language doesn't do because of the differences across platforms. The best we can do is probably something like:

/// Returns `false` if the platform implementation can definitively determine that storing the preference
/// failed. A return value of `true` indicates either that the value was written successfully, or that the
/// underlying platform API does not report success or failure.

You can't call setString with anything that isn't a string. Same with the other set methods.

Yes, those parts of the additions are not correct; the cases described would not compile, so could not throw at runtime.

[RamonFarizel]

Thanks for your thoughts, folks.

So if we are moving with updating the docs, what do you think is the best approach?

1. Should we have this message repeated for all set and remove methods?
2. Only a single message mentioning that this is useful for all those methods?

[stuartmorgan-g]

Unless something is part of the comment directly on a method, it won't show up in IDE tools, and it will be harder to find even when looking at the header manually, so far fewer people would see anything added anywhere but to each method.

[RamonFarizel]

Okay, I've implemented the changes.