Skip to content

consider adding strict_top_level_inference #836

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 3, 2024 · 11 comments · Fixed by #864
Closed

consider adding strict_top_level_inference #836

devoncarew opened this issue Dec 3, 2024 · 11 comments · Fixed by #864

Comments

@devoncarew
Copy link
Member

We should consider adding a strict_top_level_inference lint, as described here: dart-lang/sdk#59562. Note that this lint does not yet exist.

This lint would warn about every omitted return type, parameter type, and variable type of a top-level declaration or class-like-namespace-level declaration (static or instance member- or constructor declaration), which is not given a type by inference, and which therefore defaults to dynamic.

@goderbauer
Copy link
Contributor

I am in favor of this to get rid of more (implicit) dynamics.

@mosuem mosuem transferred this issue from dart-archive/lints Dec 20, 2024
@devoncarew devoncarew moved this from Triage backlog to Under consideration in Lints Triage Jan 8, 2025
@devoncarew
Copy link
Member Author

Some details:

  • originally landed in Dec 2024; it shipped in 3.7 stable
  • it’s been used in the dart_flutter_team_lints set; most feedback was around wildcards
  • it does have a quick fix?

cc @srawlins to keep me honest for the above

@natebosch, @lrhn, @chunhtai - can you review this lint? This was one we'd requested, so I'd guess mostly we'd be considering if there are any open issues to address, timing of landing it, which lint set, ... But feel free to do a general review.

@lrhn
Copy link
Member

lrhn commented Feb 25, 2025

I like it. It's a way to avoid accidental dynamic in top-level declarations.

The wildcard parameters should probably be exempt. Then 👍.

@chunhtai
Copy link

yeah I am also onboard with this, dynamic can be dangerous in some cases.

@natebosch
Copy link
Member

I am in favor of including this lint in the recommended set as it is currently implemented.

The wildcard parameters should probably be exempt. Then 👍.

I initially leaned towards exempting wildcard parameters too, but I was convinced by you that it was better not to make them exempt 😄

dart-lang/sdk#59562 (comment)

Can a wildcard parameter have an omitted type?

I'd say no. This is not about having a type inside the function, so whether the parameter can be referenced or not doesn't matter. It's about making sure that the parameter has a type in the API, and doesn't default to dynamic.

@natebosch
Copy link
Member

Oh, but we also changed course on that and wildcards are exempt now?

dart-lang/sdk@d5981ad

That's also fine and I'm happy to still include the lint as it is currently implemented 😄

@srawlins
Copy link
Member

Yes, the lint rule does have a quick fix. The lint rule shipped in Dart 3.7.

@devoncarew
Copy link
Member Author

After discussion we do want to add this to our lint sets, and into the core set (so, this will affect a package's pub score).

We also do want a bit more user feedback on this one (anything unexpected in the implementation?), so will likely ask the community to try it out.

@parlough
Copy link
Member

I'm all for this inclusion! However:

We also do want a bit more user feedback on this one (anything unexpected in the implementation?), so will likely ask the community to try it out.

Perhaps it should be in the recommended set for a cycle alongside an announcement of its eventual graduation to core?

@devoncarew
Copy link
Member Author

We also do want a bit more user feedback on this one (anything unexpected in the implementation?), so will likely ask the community to try it out.

Perhaps it should be in the recommended set for a cycle alongside an announcement of its eventual graduation to core?

This is a good idea. However, we likely wouldn't ship the next stable version of this rule set until the April / May timeframe (we try not to ship new stable version of this package more frequently than every 6 months), so we have a bit of time to get some feedback.

@parlough
Copy link
Member

However, we likely wouldn't ship the next stable version of this rule set until the April / May timeframe.

I see. Then I think a -dev release with it in core for developers to prepare for it with some small announcements to increase awareness would be sufficient :)

I could call out the dev release in the various package maintainers Discord channels and perhaps pub.dev could surface a note on the scoring page when the latest release of the lints package is a -dev release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants