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

Allow disabling lints in the analyzer for certain file patterns #34069

Open
natebosch opened this issue Aug 2, 2018 · 10 comments
Open

Allow disabling lints in the analyzer for certain file patterns #34069

natebosch opened this issue Aug 2, 2018 · 10 comments
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P4 type-enhancement A request for a change that isn't a bug

Comments

@natebosch
Copy link
Member

Since authors of codegen utilities can't know the full set of (possibly contradictory) lints that a end user has enabled we end up generating code that doesn't pass all lints and in some cases hack around this by adding a bunch of // ignore_for_file: for lints users might be using that the generated code doesn't conform to. This isn't sustainable since the set of lints is a moving target.

A much better solution would be to allow a user to configure such that, for example **.g.dart and **.template.dart files don't get lints (and maybe opt in strict checks) applied but do still get other analysis applied.

@natebosch natebosch changed the title Allow disable lints in the analyzer for certain file patterns Allow disabling lints in the analyzer for certain file patterns Aug 2, 2018
@bwilkerson
Copy link
Member

We have the ability to exclude certain files from analysis, but not to disable only lints. What analysis do you think users would want to have enabled that exclude would preclude?

@natebosch
Copy link
Member Author

I think we want to include strong mode type checking since some bugs in user written code can end up reflected as type errors in generated code.

@bwilkerson
Copy link
Member

In which case, the solution is for the user to modify something outside the generated code. Is there any way we could produce the error at the location where the change needs to be made instead of in the generated code? If so, that would be a better UX.

@natebosch
Copy link
Member Author

Yeah I think that's the ideal, and that's what we achieved with the angular plugin for the analyzer. It is a lot more work on the part of the codegen author to get there, and when they don't take those steps it's a bit worse experience for the user when they hit problems at runtime instead of analysis time.

Once the CFE has caught up and implemented all static errors that might be enough to mitigate this since at least you'd get the errors at the start of the run instead of after some code has already started...

@natebosch
Copy link
Member Author

@zoechi - how well does it work for you to include these generated files in an exclude instead of adding the ignore comments in a header?

@zoechi
Copy link
Contributor

zoechi commented Aug 2, 2018

There are still exclude related issues open, therefore havem't tried since a while.
I'm going on vacation. Not sure I'll be able to try before I leave.

@zoechi
Copy link
Contributor

zoechi commented Aug 3, 2018

@natebosch exclude didn't work.
I guess because of #25551, #29430

@zoechi
Copy link
Contributor

zoechi commented Aug 3, 2018

Dup of dart-lang/linter#320 ?

@a-siva a-siva added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Aug 14, 2018
@bwilkerson bwilkerson added the type-enhancement A request for a change that isn't a bug label Aug 28, 2018
@pq pq added the analyzer-linter Issues with the analyzer's support for the linter package label Oct 24, 2018
@DanTup
Copy link
Collaborator

DanTup commented Apr 26, 2019

It is a lot more work on the part of the codegen author to get there, and when they don't take those steps it's a bit worse experience for the user when they hit problems at runtime instead of analysis time.

To give an example of an issue I hit today - I was using json_serializable and accidentally made it generate a class named Map (it's a library for tile maps). Since the generated code is littered with Map<String, dynamic>, it resulted in a load of errors (inside the generated file). While possible, I'm not sure it's worthwhile for codegen authors to try and validate all class names being generated don't collide with things that might be in the SDK (for some uses it's fine to generate long names, but for your main data classes this would be weird).

Currently it seems like the choice is between

  • Turning off any lints that the generated code violates (most notably things like lines_longer_than_80_chars, prefer_expression_function_bodies)
    Edit: Actually, I think json_serializable is sticking to 80-chars, it's my own codegen failing at that one :-)
  • Excluding generated files from analysis and finding some errors at runtime, or only if the use in non-generated code happens to expose them

If there's another option that's less-bad than those, I'd love to know :-)

@natebosch
Copy link
Member Author

Lints might not be the only things - as we add more things like strict-* options, we might want those in our hand-written code but not our generated code.

See #30386

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-linter Issues with the analyzer's support for the linter package area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P4 type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants