It is OK to have a setter with the same name as the enclosing class

[scheglov]

This code is OK for Analyzer and VM, but Fasta produces wrong Kernel for it.

class A {
  set A(v) {}
}

main() {
  var a = new A();
  a.A = 5;
}

library;
import self as self;
import "dart:core" as core;

class A extends core::Object {
  constructor •(dynamic v) → void
    : super core::Object::•() {}
}

[eernstg]

No wrong code should be generated, of course, but given that the language specification has 'It is a compile time error if a class $C$ declares a member with the same name as $C$' (just before 10.1) it should actually not be accepted by any tool.

[peter-ahe-google]

The name of the setter is A= so it doesn't have the same name as the class.

[eernstg]

Sorry, you're right!

[peter-ahe-google]

/cc @askeksa-google

[jensjoha]

The above code currently gives an internalProblem in fasta:

Unhandled exception:
tmp.dart:2:3: Internal problem: Couldn't find 'A'.
  set A(v) {}
  ^
#0      internalProblem (package:front_end/src/fasta/problems.dart:29:3)
#1      DietListener.lookupBuilder (package:front_end/src/fasta/source/diet_listener.dart:768:14)
#2      DietListener.endMethod (package:front_end/src/fasta/source/diet_listener.dart:515:32)
#3      Parser.parseMethod (package:front_end/src/fasta/parser/parser.dart:3711:14)
#4      Parser.parseClassMember (package:front_end/src/fasta/parser/parser.dart:3582:11)
#5      Parser.parseClassBody (package:front_end/src/fasta/parser/parser.dart:3453:15)
#6      Parser.parseClass (package:front_end/src/fasta/parser/parser.dart:1499:13)
#7      Parser.parseClassOrNamedMixinApplication (package:front_end/src/fasta/parser/parser.dart:1460:14)
#8      Parser.parseTopLevelKeywordDeclaration (package:front_end/src/fasta/parser/parser.dart:509:14)
#9      Parser.parseTopLevelDeclarationImpl (package:front_end/src/fasta/parser/parser.dart:425:14)
#10     Parser.parseUnit (package:front_end/src/fasta/parser/parser.dart:309:15)
#11     SourceLoader.buildBody (package:front_end/src/fasta/source/source_loader.dart:191:14)
<asynchronous suspension>
#12     Loader.buildBodies (package:front_end/src/fasta/loader.dart:150:15)
<asynchronous suspension>
#13     KernelTarget.buildProgram (package:front_end/src/fasta/kernel/kernel_target.dart:280:20)
<asynchronous suspension>
#14     CompileTask.compile (file:///usr/local/google/home/jensj/code/dart-sdk/sdk/pkg/front_end/tool/_fasta/entry_points.dart:166:38)
<asynchronous suspension>
#15     compile.<anonymous closure> (file:///usr/local/google/home/jensj/code/dart-sdk/sdk/pkg/front_end/tool/_fasta/entry_points.dart:104:25)
<asynchronous suspension>
#16     withGlobalOptions.<anonymous closure> (file:///usr/local/google/home/jensj/code/dart-sdk/sdk/pkg/front_end/tool/_fasta/command_line.dart:364:13)
#17     CompilerContext.runInContext.<anonymous closure> (package:front_end/src/fasta/compiler_context.dart:96:35)
#18     _rootRun (dart:async/zone.dart:1126)
#19     _CustomZone.run (dart:async/zone.dart:1023)
#20     runZoned (dart:async/zone.dart:1501)
#21     CompilerContext.runInContext (package:front_end/src/fasta/compiler_context.dart:96:14)
#22     CompilerContext.runWithOptions (package:front_end/src/fasta/compiler_context.dart:107:41)
#23     withGlobalOptions (file:///usr/local/google/home/jensj/code/dart-sdk/sdk/pkg/front_end/tool/_fasta/command_line.dart:357:26)
#24     compile (file:///usr/local/google/home/jensj/code/dart-sdk/sdk/pkg/front_end/tool/_fasta/entry_points.dart:97:18)
<asynchronous suspension>
#25     compileEntryPoint (file:///usr/local/google/home/jensj/code/dart-sdk/sdk/pkg/front_end/tool/_fasta/entry_points.dart:55:11)
<asynchronous suspension>
#26     main (file:///usr/local/google/home/jensj/code/dart-sdk/sdk/pkg/front_end/tool/_fasta/compile.dart:7:33)
#27     _startIsolate.<anonymous closure> (dart:isolate-patch/dart:isolate/isolate_patch.dart:275)
#28     _RawReceivePortImpl._handleMessage (dart:isolate-patch/dart:isolate/isolate_patch.dart:163)

[peter-ahe-google]

I consider known crashes to be P1.

Aske, could you take a look?

[lrhn]

We're probably going to fix that loop-hole. It's not helping anyone to be able to declare a setter and not be allowed to declare the corresponding getter.
I'm thinking about defining the "base name" of a member name in such a way that the base name of x= is x, and saying that a class must not declare a member with the same base name as the class name (and not declare two members with the same base name unless one is a getter and the other a setter, and the base name of a named constructor is the part after the ., etc.).

[peter-ahe-google]

@lrhn, I agree. But from the perspective of Fasta, this is just another case of duplicated names, and we have to handle those gracefully anyways. We can't stop code completion just because there's a duplicated member. Sometimes, the way we recover from parser errors also makes us think there's a duplicated member.

For example:

class A {
  const A();
}

A() // @ is missing here.
main() {}

We currently recover from this code by inventing a top-level method A().

[eernstg]

@lrhn wrote:

We're probably going to fix that loop-hole.

One of the things implied by this CL is that it is an error to have a setter (instance or static) whose basename is the same as the name of the enclosing class, which is basically a revert on the CL that closed this issue (https://dart-review.googlesource.com/34120). A 'basename' is the same as the name for all members, except that it strips off the = at the end of a setter name.

It is also the consistent choice, considering all the other rules about class member name conflicts.

WDYT, would it break any significant amount of existing code to revert the CL that closed this issue (with adjustments as needed, because other things have changed since then)?