Skip to content
This repository was archived by the owner on Nov 20, 2024. It is now read-only.

Conversation

@asashour
Copy link
Contributor

Covers the cases for dart-lang/sdk#58453

I am not sure about the documentation request, since this is a test case

m3(f(int)) => null; // LINT

@google-cla google-cla bot added the cla: yes label Jul 28, 2021
@coveralls
Copy link

coveralls commented Jul 28, 2021

Coverage Status

Coverage increased (+0.0005%) to 95.82% when pulling 2006fb3 on asashour:2770_avoid_types_as_parameter_names into ec373f4 on dart-lang:main.

Copy link
Contributor

@bwilkerson bwilkerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that looks good, thanks.

Copy link
Contributor

@scheglov scheglov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pq pq added linter-set-core Affects a rule in the core Dart rule set and removed set: core labels Jul 18, 2022
@srawlins
Copy link
Contributor

I'd be happy to land this, but I'll need conflicts resolved in order to test inside Google first.

@asashour
Copy link
Contributor Author

Conflicts resolved, thanks.

@srawlins
Copy link
Contributor

Thanks!

Comically, there are a lot of breakages internally; 99% of them are parameters of the form int num, which is for some reason triggered by this PR.

@bwilkerson
Copy link
Contributor

If there are both a type and a name, then it seems reasonable that this lint not fire. As I recall, the motivation for this lint was to prevent cases where someone wrote a type name and forgot to write a parameter name, which clearly isn't the case if the parameter name is there.

@srawlins
Copy link
Contributor

In that case, we might be able to just revert the deleted declaredElement.hasImplicitType && line, and continue.

(There were other breakages in google3 I'll clean up first)

@srawlins
Copy link
Contributor

srawlins commented Nov 1, 2022

Ah, now we should change those test cases to have implicit parameter types, if possible. I think the function-typed ones cannot have implicit types. So in void f(int) {}, I've created a parameter named int with type dynamic, but in a function type like void f(void Function<T>(T) callback) {}, there is no parameter named T, there is only an unnamed parameter with type T.

@asashour
Copy link
Contributor Author

asashour commented Nov 1, 2022

I see, PTAL.

@srawlins
Copy link
Contributor

I'm still working on cleaning this one up internally.

@srawlins
Copy link
Contributor

We are now clean internally.

@pq
Copy link
Contributor

pq commented Dec 12, 2022

@srawlins: did you happen to look at Flutter too? (No worries either way...)

@srawlins
Copy link
Contributor

Not yet. I hope have a moment this week. 🤞

@pq
Copy link
Contributor

pq commented Dec 12, 2022

@srawlins: you asked for a change... I think it's done?

@srawlins
Copy link
Contributor

Yes, the one change I requested is done.

Copy link
Contributor

@pq pq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@pq pq merged commit 2d246a5 into dart-archive:main Dec 12, 2022
@asashour asashour deleted the 2770_avoid_types_as_parameter_names branch December 12, 2022 21:48
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Aug 23, 2023
…linter#2823)

* Fix false negative in `avoid_types_as_parameter_names`

* use visitGenericFunctionType

* Remove visitGenericFunctionType

* Merge and removed sections done for a now fixed issue.

* Lint for implicit types.

* Make tests has implicit parameter types
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes linter-false-negative linter-set-core Affects a rule in the core Dart rule set

Development

Successfully merging this pull request may close these issues.

6 participants