Code clean up, additional lints

[josxha]

• This pull request makes use of opinionated lints to improve code style and code consistency.
• It chages normal comments to documentation comments to improve the IDE support
• Use the @immutable annotation to make it clear what classes are completely immutable and use const and final where possible
• Make it possible to in instanciate lib\src\layer\label.dart
• Add messages to asserts
• Fix the one time computation of of some lazy fields
• Some performance improvements

Most of the changes are refactoring. I tested it with the demo application and haven't experienced any problems.

---

Lint references:
https://dart.dev/tools/linter-rules/use_super_parameters
https://dart.dev/tools/linter-rules/cast_nullable_to_non_nullable
https://dart.dev/tools/linter-rules/only_throw_errors
https://dart.dev/tools/linter-rules/sort_unnamed_constructors_first
https://dart.dev/tools/linter-rules/prefer_asserts_with_message
https://dart.dev/tools/linter-rules/missing_whitespace_between_adjacent_strings
https://dart.dev/tools/linter-rules/avoid_multiple_declarations_per_line
https://dart.dev/tools/linter-rules/noop_primitive_operations
https://dart.dev/tools/linter-rules/avoid_void_async
https://dart.dev/tools/linter-rules/avoid_redundant_argument_values
https://dart.dev/tools/linter-rules/avoid_types_on_closure_parameters
https://dart.dev/tools/linter-rules/omit_local_variable_types
https://dart.dev/tools/linter-rules/matching_super_parameters
https://dart.dev/tools/linter-rules/unnecessary_null_checks
https://dart.dev/tools/linter-rules/prefer_single_quotes
https://dart.dev/tools/linter-rules/unnecessary_parenthesis
https://dart.dev/tools/linter-rules/use_if_null_to_convert_nulls_to_bools

[TesteurManiak]

Thanks for the contribution I've just reviewed the first modified files but here's some of my concerns:

• While it's nice to have more opiniated linting rules and uniformize the coding style you should've probably discussed those beforehand with one of the maintainers to establish clear rules to add to the CONTRIBUTING.md file.
• I find useless to specify every time you create a list growable: false it doesn't bring any performance improvement nor warnings/errors from the analyzer before execution of the code, personally I find it unneeded as if we really wanted to have an immutable list we could've used directly an IList object from fast_immutable_collections or dartz
• Using => for fast return might feel faster because you're saving some lines of code but from my experience it reduces readability and it is preferable to keep brackets with an explicit return
• About the linting rules you've added there's 2 that I find to be too bothersome when coding: omit_local_variable_types because I wouldn't be able to write a variable like this double myVar = 1.0 it would expect a var myVar = 1.0 but I find it to reduce type safety and the fact that you've ignored this rule by yourself in latlng_bounds.dart makes me think that it shouldn't be part of the linting rules, prefer_single_quotes has also proven to me to be a bit too much opiniated as I like to use double quotes when I've got characters such as ' in my string instead of writing \'.

Other than that the other linting rules seems fair to me and some refacto you've been doing are much appreciated and improve code readability but you probably should have opened an issue explaining the changes you had in mind to improve the coding style so it could've been discussed with maintainers and contributors before starting this draft. (would love opinions of other maintainers on this btw @JaffaKetchup @ibrierley @mootw)

[JaffaKetchup]

I'll reply a little more a little later, but just for now I'll say that 'prefer_single_quotes' does still allow double quotes when single quotes are inside the string @TesteurManiak.

[TesteurManiak]

I'll reply a little more a little later, but just for now I'll say that 'prefer_single_quotes' does still allow double quotes when single quotes are inside the string @TesteurManiak.

You're right! My bad then, this rule can be kept 👍

[josxha]

Thank you that you guys already had a look through the pr, @TesteurManiak @JaffaKetchup! I wanted to dive deeper into the flutter_map code and took the chance to do some minor improvements on the way.

• While it's nice to have more opiniated linting rules and uniformize the coding style you should've probably discussed those beforehand with one of the maintainers to establish clear rules to add to the CONTRIBUTING.md file.

I knew that introducing more lints will lead to discussions and I'm completely fine to make changes to this pull request like removing lint rules. (: As mentioned the lint rules are opinionated and should defently get discussed. The lints that I added are purely meant to ensure code consistency and code readability.

• I find useless to specify every time you create a list growable: false it doesn't bring any performance improvement nor warnings/errors from the analyzer before execution of the code, personally I find it unneeded as if we really wanted to have an immutable list we could've used directly an IList object from fast_immutable_collections or dartz

I'm a bit confused because making Lists immutable wasn't my focus. I added growable: false 2 times when transforming Lists but that's just my default coding style. The claim that it's brings no benefit is not completely right as it indeed brings some memory improvement and may bring some platform specific performance improvements. (Backing my statements on this https://stackoverflow.com/a/65842740/9439899)

• Using => for fast return might feel faster because you're saving some lines of code but from my experience it reduces readability and it is preferable to keep brackets with an explicit return

No indeed there is no performance improvements here. I idea behind it is a reduce nesting. But I can revert those changes (especially in flutter_map_interactive_viewer.dart) if you like.

• About the linting rules you've added there's 2 that I find to be too bothersome when coding: omit_local_variable_types because I wouldn't be able to write a variable like this double myVar = 1.0 it would expect a var myVar = 1.0 but I find it to reduce type safety and the fact that you've ignored this rule by yourself in latlng_bounds.dart makes me think that it shouldn't be part of the linting rules,

var myVar = 1.0 will be a double because or the .0 but I agree that this lint has some problematic szenarious. Dart wants you to write get rid of the leading .0 because it seams unnecessary (not sure if it's the IDE or a default lint rule tho). It is still completely type safe but the problem is that it then gets initialized as int and use it to store a double afterwards any more.

But I can defently remove that lint as it might complicate things.

prefer_single_quotes has also proven to me to be a bit too much opiniated as I like to use double quotes when I've got characters such as ' in my string instead of writing \'.

I completely agree on that, but the thing is that flutter_map is currently 100% minus one szenario written with single quotes!😄 So I thought why not add this rule for concistency in the future. And like @JaffaKetchup said it doesn't prevents you to use double quotes when you're using a single quote inside.

you probably should have opened an issue explaining the changes you had in mind to improve the coding style

I keep that it mind for furture contributions, thanks! Don't worry if you need to revert things in this draft pull request as it got created while i dug through the code. I'm completely open to make changes and hear about possible concerns from the other maintainers. Just because I created a pull request directly doesn't mean that I don't want to open it up for discussion. (:

[JaffaKetchup]

I think I prefer specifying the local variable types than using var, just for clarity, although where something is obvious, such as a string, I don't really mind

I ALWAYS use => wherever is physically possible, including in build methods, against the recommendations of the Flutter style guide. However, I recognise that I am on the extreme end here, and so I'll side with @TesteurManiak here that => should only be used for a single or double line return.

I've never specified growable: false in toList, but I don't mind either way. I guess it might bring some memory improvements. I only ever use it when doing something like List.generate().

[josxha]

I ALWAYS use => wherever is physically possible, including in build methods, against the recommendations of the Flutter style guide. However, I recognise that I am on the extreme end here, and so I'll side with @TesteurManiak here that => should only be used for a single or double line return.

I don't really get why => is such a problem in this pr since there are methods like polyline_layer.dart#L44C3-L55C2 and similar in the code base but it doesn't matter to me. Please note that changes in flutter_map_interactive_viewer.dart had the goal to give the parameter instance a more meaninful name and not to convert method bodies to the => notation. That's why I'm completely fine to change the method bodies back to not use =>.

[JaffaKetchup]

@TesteurManiak Can you review this, as you started reviewing first?

[TesteurManiak]

Sorry for the delay, I'll try to review changes as soon as I can 😓

[TesteurManiak]

LGTM 🎉