Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why does effective_dart cancel out the hints from pedantic? #2029

Closed
andesappal opened this issue Mar 13, 2020 · 14 comments
Closed

Why does effective_dart cancel out the hints from pedantic? #2029

andesappal opened this issue Mar 13, 2020 · 14 comments

Comments

@andesappal
Copy link

I had 87 hints from pedantic. Then I added effective_dart, and the hints disappeared.

@andesappal
Copy link
Author

Also, could your team just automate the correction of pedantic hints? Or display an option to correct all hints?

@andesappal
Copy link
Author

andesappal commented Mar 13, 2020

This is what I have:
image

I also noticed I have a warning "Key 'include' is duplicated" in
image

Before I saw this warning, was using pedantic 1.9.0 and effective 1.2.1

@bwilkerson
Copy link
Member

We don't currently support including more than one other file in an analysis options file (this is the first use case I've heard of for it). There are some interesting questions about how the files would interact in case of conflicts that we decided to solve by disallowing that situation to arise.

My guess is that the second include replaces the first.

@andesappal
Copy link
Author

Disallowing it doesn't solve it, right? But that's ok, I'll use one analysis option at a time. Thank you.

@andesappal
Copy link
Author

Maybe what's effective implies new code, thereby invalidating the current pedantic hints. I see, it does pose an interesting problem.

@bwilkerson
Copy link
Member

The problems related to multiple includes are around conflicts. If one says to enable a lint and another says to disable it, which one wins? With a single include those problems don't exist.

And we defined the "conflicts" between a file and the file it includes to be resolved by saying that the file containing the include always takes precedence.

@domesticmouse
Copy link
Member

Can we get this re-opened? Being able to depend on both of the pedantic and effective_dart linters is really needed. Last one wins, potentially with a warning. Or even make it an error.

@bwilkerson bwilkerson reopened this Jan 11, 2021
@bwilkerson
Copy link
Member

I've re-opened the issue, but I'm not sure what you're asking for. We produce a diagnostic (with a severity of error) telling the user that we don't support having multiple includes (or, more technically, the YAML parser tells the user that duplicate keys are not allowed).

Being able to depend on both of the pedantic and effective_dart linters is really needed.

I'd be interested to understand more about why this is needed and just how badly the lack of support is impacting our users. I'm not opposed to adding support, but we need to prioritize our efforts.

Unfortunately, supporting the inclusion of multiple analysis options files is a non-trivial effort. If we only needed to worry about lints, then I suspect that the merge semantics would be fairly easy to define. But we're including the whole analysis options file, and the semantics of merging all of the options isn't quite as straight-forward. (We actually spent some time when first adding inclusion support attempting to do just that and decided that it was more important to be able to have a single chain of inclusion than to postpone the release while we figured it all out.)

@domesticmouse
Copy link
Member

The core issue is that the two list of lints contain disjoint recommended lints. I want to be able to recommend a best practices setting that contains both.

One work around would be to merge the two sets of lints into a best practices set and publish that list.

@bwilkerson
Copy link
Member

@mit-mit

@mit-mit
Copy link
Member

mit-mit commented Jan 12, 2021

@domesticmouse can you elaborate on why this is really needed? I'm not sure I understand the use case.

@domesticmouse
Copy link
Member

Let me turn the question around the other way. Why are there lints included in the effective_dart configuration, which AFAICT enforce the best practices we document at dart.dev/guides/language/effective-dart, but are not int the pedantic configuration?

@bwilkerson
Copy link
Member

The pedantic package represents the collection of lints that are enabled for our internal users. The internal community has not chosen to adopt all of the lints in effective_dart for various reasons. Sometimes it's because of the number of false positives, sometimes for other reasons.

@domesticmouse
Copy link
Member

False positives are bad. I'll stick with just recomending pedantic then.

@mit-mit mit-mit closed this as completed Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants