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.

Sign up for GitHub

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

Jump to bottom

[style] update analyzer options to the recommended lint set #48784

Open
2 of 9 tasks
pq opened this issue Apr 11, 2022 · 5 comments
Open
2 of 9 tasks

[style] update analyzer options to the recommended lint set #48784

pq opened this issue Apr 11, 2022 · 5 comments
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) P3 A lower priority bug or feature request type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable

Comments

@pq
Copy link
Member

pq commented Apr 11, 2022 •
edited by srawlins
Loading

Transitioning from pedantic to recommended lints there are a number of lints disabled that need vetting.

  • avoid_renaming_method_parameters
  • camel_case_types
  • constant_identifier_names
  • hash_and_equals
  • non_constant_identifier_names
  • implementation_imports
  • library_private_types_in_public_api
  • overridden_fields
  • provide_deprecation_message
@lrhn lrhn added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label Apr 11, 2022
@pq pq added the type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable label Apr 11, 2022
@pq pq mentioned this issue Apr 11, 2022
Open
12 tasks
copybara-service bot pushed a commit that referenced this issue Apr 11, 2022
@pq
migrate analyzer to recommended lints
e0d87b8 
See: #48784

Change-Id: I69a2b6ef16159796e5fa405308438df62af6c826
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/240905
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
@scheglov scheglov added the P3 A lower priority bug or feature request label Apr 13, 2022
@srawlins srawlins added the contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) label May 20, 2022
copybara-service bot pushed a commit that referenced this issue Aug 10, 2022
@srawlins
Tidy some lint issues in analyzer
e139fb1 
* Move many identifiers from snake_case or SCREAMING_SNAKE_CASE to
  camelCase. I think I restricted the changes to private API.
* Add an ignore in generated files for constant_identifier_names;
  we often just mirror names found in YAML files or whatever, where
  things are not necessarily named according to Effective Dart
  standards.
* Avoid some implementation imports which I think were accidental.
* Use single quotes in some more places.

Bug: #48784
Change-Id: I439fe50cb9f8f8f5e9c74ce594aea15e394ee7da
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254500
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
@asashour

This comment was marked as off-topic.

Sign in to view
@bwilkerson
Copy link
Member

This issue appears to be a request to decide which of the listed lints we should enforce in the analyzer packages. I think we should make those decisions before we attempt to enforce the lints.

That said, the CLs above appear to be enforcing type_annotate_public_apis, which isn't on the list. While I believe that we should enable that lint (we follow it 90+% of the time, and consistency in this area would be a good thing imo), that also hasn't been decided.

@asashour

This comment was marked as off-topic.

Sign in to view
@pq
Copy link
Member Author

pq commented Sep 2, 2022

This issue appears to be a request to decide which of the listed lints we should enforce in the analyzer packages.

Also FYI @devoncarew who is in conversations about defining (and enforcing) a dart team lint set (which may or may not derive from the current recommended rules).

@devoncarew
Copy link
Member

FWIW, a dart team rule set - once defined - would definitely be strict super-set of package:lints/recommended.yaml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) P3 A lower priority bug or feature request type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@pq @srawlins @scheglov @devoncarew @asashour @lrhn @bwilkerson