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

[linter] Support for non-function type aliases #58347

Closed
8 tasks done
pq opened this issue Mar 15, 2021 · 7 comments
Closed
8 tasks done

[linter] Support for non-function type aliases #58347

pq opened this issue Mar 15, 2021 · 7 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. linter-new-language-feature type-enhancement A request for a change that isn't a bug

Comments

@pq
Copy link
Member

pq commented Mar 15, 2021

Follow-up from #44078 and #44951, we need to shore up linter support for non-function type aliases.

@pq
Copy link
Member Author

pq commented Mar 18, 2021

I did a quick look through existing lints and besides the fix to prefer_mixin didn't come up with anything else to address. I didn't feel a strong pull to add test cases either though I'm super open to suggestions.

@srawlins, @bwilkerson, @scheglov : curious if anything jumps out at you?

As for new lints, I'm curious if there's any thinking about naming conventions. CamelCase? If yes, then thoughts on extending camel_case_types? Alternatively I guess we could add camel_case_nonfunction_typedefs 😬 ...

@bwilkerson
Copy link
Member

Doesn't camel_case_types already cover the names defined via typedef? I would expect it to just continue to work, but a test for that case would be in order.

As for other places that might need fixed up, you could search for places where staticElement is being invoked on a TypeAnnotation, though that's probably going to return lots of places where it's used on something other than a type annotation.

@pq
Copy link
Member Author

pq commented Mar 18, 2021

Doesn't camel_case_types already cover the names defined via typedef?

Yes it does cover function type aliases.

The question is whether it should flag the following:

class A { }

typedef a = A;

I think yes?

(Currently it does not.)

Thanks for the tip on staticElement greps!

@bwilkerson
Copy link
Member

Ah, that's because it's visiting FunctionTypeAliass when I think it ought to be visiting GenericTypeAliass (it might need to visit both until the experiment is enabled). So you can also look for other implementations of visitFunctionTypeAlias.

@scheglov
Copy link
Contributor

scheglov commented Mar 18, 2021

FWIW, this means that the lint did not work for "modern" function type aliases as well, like typedef b = void Function();.
FunctionTypeAlias is not going anywhere, I think, because it is a different syntax than GenericTypeAlias.
FunctionTypeAliasElement is deprecated, and will be removed.

@pq
Copy link
Member Author

pq commented Mar 18, 2021

Looking at visitFunctionTypeAlias, brought up these suspects:

  • avoid_private_typedef_functions
  • public_member_api_docs
  • slash_for_doc_comments

All seem straightforward. Thanks!

@pq
Copy link
Member Author

pq commented Mar 19, 2021

Marking as done. If ideas for lints present themselves we can open new tracking issues.

@pq pq closed this as completed Mar 19, 2021
@devoncarew devoncarew added 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. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-lang/linter Nov 19, 2024
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. linter-new-language-feature type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants