search

dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
636 stars 170 forks source link

avoid_renaming_method_parameters doesn't fire #1786

Closed davidmorgan closed 5 years ago

davidmorgan commented 5 years ago

The lint doesn't fire anywhere in google3, and I couldn't get it to fire by hand, even using the example in the lint docs. Any ideas please?

abstract class A {
  m(a);
}

abstract class B extends A {
  m(b);
}
pq commented 5 years ago

/cc @a14n

a14n commented 5 years ago

I can see the lint on your example. What's the version of Dart SDK?

davidmorgan commented 5 years ago

Thanks! I dug in a bit more. The lint (using dartanalyzer --lint on the command line) only works if there is a pubspec.yaml file.

Since we don't use pubspec.yaml in google3 it's not there and the lint doesn't fire.

Question is, what is it in the implementation that needs it? I don't see anything. Maybe @pq has an idea...

pq commented 5 years ago

Right. We use the pubspec here as a way to identify the root of the project.

https://github.com/dart-lang/linter/blob/1394a375eadf91433e7499e630204b70634742d6/lib/src/ast.dart#L125-L131

which, in turn, is how we identify if we're in lib/

https://github.com/dart-lang/linter/blob/1394a375eadf91433e7499e630204b70634742d6/lib/src/rules/avoid_renaming_method_parameters.dart#L83

This is a a kind of funky hack and there are a few other ways to do it. I'll take a look at alternatives today.

pq commented 5 years ago

The current thinking is to update our test to use workspaces which are Google3-aware.

Supporting API update in flight here:

https://dart-review.googlesource.com/c/sdk/+/123341

pq commented 5 years ago

@davidmorgan: this should be fixed as of the next roll into Google3; please let me know if you see otherwise!