Skip to content
This repository has been archived by the owner on Feb 12, 2023. It is now read-only.

Disable omit_local_variable_types rule #4

Closed
trietbui85 opened this issue Oct 28, 2019 · 6 comments
Closed

Disable omit_local_variable_types rule #4

trietbui85 opened this issue Oct 28, 2019 · 6 comments
Labels
feedback wanted Looking for feedback from the community

Comments

@trietbui85
Copy link

I think we could disable omit_local_variable_types by default.

At the moment, below code will cause a warning

User user = repo.getUser();

But this one will not:

var user = repo.getUser();

However, User user = ... will make code easy to understand, and if anyone needs to view User source code, he can press Ctrl and click on User to navigate to User class.
I believe after compiling, such var will be replaced by the correct type (User).

Thus, I think we should disable omit_local_variable_types by default.

@tenhobi
Copy link
Owner

tenhobi commented Oct 28, 2019

Hi @anticafe ✋, thanks for opening an issue!

This lint follows this guide's rule. I think this is a good rule, because of the reason mentioned there.

However, if you want to use most of the other Effective Dart lint rules, you can disable this rule by yourself, as mentioned in #3. (I will enhance README later today)

Of course you can share your opinion on this, thanks!

@tenhobi tenhobi added the wontfix This will not be worked on label Oct 28, 2019
@tenhobi
Copy link
Owner

tenhobi commented Oct 28, 2019

Or, if you want to use this lint in most of the cases, you can also supress this rule for specific file or line.

See https://dart.dev/guides/language/analysis-options#suppressing-rules-for-a-file or https://dart.dev/guides/language/analysis-options#suppressing-rules-for-a-line-of-code

@tenhobi tenhobi added the waiting for response Waiting for follow up label Oct 28, 2019
@trietbui85
Copy link
Author

Thanks for your response @tenhobi
Yes in my project, I always set omit_local_variable_types: false in analysis_option.yaml. What I suggestion here is should we disable it by default in this package.

For me, depend on the name of method on the right-hand side then I would use var or correct type for variable. For example: var user = repo.getUser();, but Map<Organization, User> orgAndUser = repo.getOrgData();.

@tenhobi tenhobi added feedback wanted Looking for feedback from the community and removed waiting for response Waiting for follow up wontfix This will not be worked on labels Oct 28, 2019
@tenhobi
Copy link
Owner

tenhobi commented Oct 28, 2019

OK, I see the issue. This lint is good, but there are cases where it is not required. Now there is a question of what is better: disabling or suppressing.

We might want to wait for some response from the comunity.

@felangel can you give your opinion? You modified a lot of code.

@tenhobi
Copy link
Owner

tenhobi commented Oct 28, 2019

There are project who do and do not use this rule. In flutter it's disabled, in e.g. https://github.com/google/flutter-desktop-embedding it's enabled. Dunno what to choose.

@tenhobi tenhobi changed the title Please disable omit_local_variable_types rule Disable omit_local_variable_types rule Oct 28, 2019
This was referenced Oct 28, 2019
@tenhobi
Copy link
Owner

tenhobi commented Oct 29, 2019

Hi, first of all, thanks again for opening this issue. 🎉

As mentioned in #6, we will not disable rules if they work properly according to the rules and across all tools. Only exception might be lines_longer_than_80_chars.

I know there are many rules which might not always suit your project needs, but this package should primarily cover Effective Dart rules. And you can always suppress those lints you don't want to follow, as mentioned in the README. 👍

@tenhobi tenhobi closed this as completed Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feedback wanted Looking for feedback from the community
Projects
None yet
Development

No branches or pull requests

2 participants