Different errors in Analyzer and CFE for constructor and static member with the same name

[sgrekhov]

If class contains constructor and static member with the same name, then analyzer produces one error but CFE produces two ones.

class C {
  C.s() {}  // error in analyzer and CFE
  static set s(var val) {} // error in CFE only
}

There is one error in analyzer here (on constructor) error - 's' can't be used to name both a constructor and a static field in this class.

And there are two errors in CFE Error: Conflicts with constructor 's'. Error: Conflicts with setter 's'.

Shouldn't analyzer and CFE behave in a same way here?

Tested on Dart SDK version: 2.14.0-374.0.dev (dev) (Mon Aug 2 14:07:46 2021 -0700) on "windows_x64"

[lrhn]

The language specification doesn't require a specific way to represent a compile-time error, so saying the same thing in different ways is not wrong. (I'm personally undecided about which one is better. I like that the CFE points out both places, so I won't have to manually search for the conflicting "field", which is actually a setter).

[vsmenon]

@johnniwinther @srawlins

I'm inclined to say this is working as intended. Do you have opinions on whether we want to be more consistent? And how we want to track consistency issues that aren't necessarily a bug on one or the other?

[bwilkerson]

Do you have opinions on whether we want to be more consistent?

You didn't ask me, but yes, I have an opinion. :-) I believe that having consistent messaging across our tools for any reported problem is helpful to users in correlating the results they're seeing from the tools. Using the example from this issue, if both tools were to report that the setter conflicts with the constructor then users would easily be able to see that there's only one problem. But if one reports a problem with the setter and the other reports a problem with the constructor, then it takes more effort for the user to determine that (a) the tools are consistent and (b) that there's a single problem. The first part is important because it impacts user trust; the second because it impacts productivity.

Of course, consistency shouldn't be an end in itself, it should be a means to an end, where the end is to improve the quality of the tools. In cases where one tool is able to provide higher quality messaging than the others then we shouldn't sacrifice quality for the sake of consistency.

That said, my impression is that the tools are a long way from being consistent and we probably need to have a concerted effort to make them as consistent as possible, recognizing that the differences in the way the tools currently work might make consistency impractical in some cases. Continuing to move closer to having a unified front-end would decrease the number of places where consistency isn't practical. I believe that an effort like this was proposed last quarter on the language team's OKRs but didn't make the cut. I personally believe that it needs to be a cross team effort. (@leafpetersen @stereotype441)

I'm personally undecided about which one is better.

I feel strongly that there should be exactly one diagnostic per problem and that there is only one problem here: a name is being used for two different declarations in a way that is ambiguous.

I like that the CFE points out both places, so I won't have to manually search for the conflicting "field", which is actually a setter.

So do I, but we have another mechanism (context messages) that would be a much better way to surface this information to users. By making the additional information available as a context message we make the correspondence of the two pieces of information explicit.

[vsmenon]

Thanks, @bwilkerson :-)

In the interests of triaging / tracking, any thoughts? We should have some bucket for consistency issues, but I don't think we do right now.

[bwilkerson]

A label sounds like a reasonable first step to me. We can do something more formal if a need becomes apparent.

[lrhn]

I feel strongly that there should be exactly one diagnostic per problem and that there is only one problem here: a name is being used for two different declarations in a way that is ambiguous.

If that diagnostic can only point to one point in the program, then it's less useful (and arbitrary) than one which points to both declarations which are part of the problem. If we can have one diagnostic with two program locations, then that's fine. Picking either one is arbitrary.

[vsmenon]

Ok, I've created the label front-end-analyzer-unification for consistency issues and/or further unification plans.