Overlapping generic type and function parameter names should not be allowed

[jakemac53]

Dart SDK Version: 2.0.0-edge.6ed3b162d66a3f261ba3f5bc8ce430fefcbef4a0 (Fri May 25 16:05:20 2018 +0000) on "linux_x64"

Today, it is possible to write this program:

main() {
  print(foo(1));
}

/// Note the function parameter has the same name as the Generic type parameter!
T foo<T>(T) => T;

This will print 1 on the vm and dart2js, and ddc will currently generate code with a syntax error.

I think this should be a static error recognized by the analyzer/front_end, and never attempt to run. I can't think of a scenario in which it does what the user intended, and it could be difficult to diagnose.

cc @kevmoo @jmesserly

[jakemac53]

It looks like there is actually a lint for this avoid_types_as_parameter_names, which might be enough for now (although it wouldn't be added by default for any projects which isn't great).

If its not going to become a compile time error we should fix ddc though.

[kevmoo]

Agreed!

[leafpetersen]

I don't see any real issues with doing this (at least for generic method parameters), but I don't think it's high enough priority to get onto the front end teams TODO list for Dart 2. Maybe live with a lint for now and aim to restrict it in a future cleanup?

[lrhn]

For classes, we disallow members with the same name as type parameters, so there is precedence for making restrictions on this that are not technically necessary.

There is no technical reason to prohibit the code. The type parameter scope is the parent scope of the parameter scope, so we are not declaring the variables in the same scope. The only issue is whether that matches the user's perception, and whether making the restriction will catch more errors without preventing useful programs. I'd say that it seems so, there is no good code that uses a single capital letter as parameter name (unless it's generated code?), and it does catch the error of writing var x; foo<T>(T /*forgot to write argument name "x" */) { ... x /* now hits outer x */}. That's not significantly different from just mistyping the variable name, though, which we still won't catch, or will still catch with a "unused variable" warning.

[eernstg]

I actually suspect it will be more helpful for developers to have lints in place that help enforcing simple naming conventions (like upper/lower case): This will catch the missing parameter x as well as "ugly code" where the scoping is OK but it just violates the conventions. Then we don't need more funny exceptions to the normal scope rules. ;-)

[jakemac53]

There is no technical reason to prohibit the code. The type parameter scope is the parent scope of the parameter scope, so we are not declaring the variables in the same scope. The only issue is whether that matches the user's perception, and whether making the restriction will catch more errors without preventing useful programs.

Yes, this is purely about helping the user with early static errors for a scenario that I think indicates a user mistake in pretty much all scenarios.

I actually suspect it will be more helpful for developers to have lints in place that help enforcing simple naming conventions (like upper/lower case):

Agreed, and these lints do already exist (non_constant_identifier_names, camel_case_types). Interestingly these were both enabled on the repo which actually ran into this issue, and it wasn't caught by them so maybe there is a bug in the lints.

The main problem with lints in general though is they are opt-in, and aren't very discoverable.