Skip to content

The non_constant_identifier_names lint should ignore a leading underscore #58288

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

Closed
devoncarew opened this issue Dec 16, 2020 · 7 comments
Closed
Assignees
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-false-positive type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@devoncarew
Copy link
Member

The non_constant_identifier_names should likely ignore a leading underscore. For methods names like void _checkAssignment_recursion(), I see:

info: Name non-constant identifiers using lowerCamelCase. (non_constant_identifier_names at [nnbd_migration] lib/src/edge_builder.dart:3313)

Also, the code for the lint is non_constant_identifier_names, which seems like it describes the items it applies to, instead of what it does (like 'prefer_lower_camel_case').

@bwilkerson
Copy link
Member

I agree that a leading underscore should be ignored.

Side note on naming: Yes, it's unfortunate that the lint names don't all conform to a standard naming scheme. If it weren't for the pain it would cause users I'd suggest renaming them all more consistently, but I can't convince myself that it would be a net positive from a user perspective. But we realized awhile back that we'd made a mistake by using "prefer", "avoid" and other terms from the style guide because the style guide has, in a few cases, changed since the lints were originally named and now the names are inconsistent.

@pq pq self-assigned this Dec 16, 2020
@pq
Copy link
Member

pq commented Dec 16, 2020

The issue w/ lint naming inconsistency bothers me daily. I really wish we'd been more principled and consistent but as Brian points out, our ideas have evolved. Thinking ahead, it'd be cool to do a renaming at some point but so far the pain hasn't seemed worth it. (Maybe we can revisit if bulk fixing takes off?) (FWIW there's some related discussion in #57794.)

As for this lint misfiring, my sense is that it should be further expanded to allow for underscores in function declarations too (and am somewhat baffled that it hasn't already...) Thoughts?

@pq pq added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) linter-false-positive labels Dec 16, 2020
@devoncarew
Copy link
Member Author

If there are a few lint names that really stand out, we could rename them to get a better sense for the cost to the ecosystem. We'd likely want to do something like have two names for the lint - the new and the deprecated old - show a deprecation messages, and possibly, have a quick fix to convert from the old to the new.

@bwilkerson
Copy link
Member

That sounds like a reasonable path forward. Perhaps Phil and I can carve out a few hours to review the lint names at some point to see what the magnitude of the changes might be.

@pq
Copy link
Member

pq commented Dec 16, 2020

SGTM!

@pq
Copy link
Member

pq commented Dec 16, 2020

Taking a closer look, the issue here is not the leading underscores, it's what follows. Specifically _recursion(). To fix it, consider renaming to: _checkAssignmentRecursion().

@pq
Copy link
Member

pq commented Dec 16, 2020

Closing as working as intended.

@pq pq closed this as completed Dec 16, 2020
pq referenced this issue in dart-archive/linter Dec 16, 2020
pq referenced this issue in dart-archive/linter Dec 16, 2020
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-false-positive type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants