Skip to content

It is an error to have a setter with the same name as the enclosing class #34225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
eernstg opened this issue Aug 22, 2018 · 8 comments
Closed
Assignees
Labels
front-end-missing-error legacy-area-front-end Legacy: Use area-dart-model instead. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@eernstg
Copy link
Member

eernstg commented Aug 22, 2018

A class name can clash with the basename of an instance or static setter (cf. 9085009):

class C {
  static set C(v) {} //# 01: compile-time error
  set C(v) {} //# 02: compile-time error
}

dart (version 2.1.0-dev.0.0) reports an error for 01 (but not the correct one, it says Constructors can't be static) and accepts 02. dartanalyzer (version 2.1.0-dev.0.0) does not report an error for 01 nor 02.

Note that this is a near-revert of 402e69b which was used to close #30834: This error used to be emitted until that commit.

This means that a fix to this issue introduces an error which is a breaking change, but it was also an error until Feb 6, 2018, so the breakage is not likely to be large.

@eernstg eernstg added legacy-area-analyzer Use area-devexp instead. legacy-area-front-end Legacy: Use area-dart-model instead. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Aug 22, 2018
@matanlurey
Copy link
Contributor

Is this a duplicate of (or overlap with) #32968 @eernstg?

@eernstg
Copy link
Member Author

eernstg commented Aug 22, 2018

No, #32968 is concerned with a conflict where a superclass declares a static member and a subclass declares a member (instance or static) with the same basename. This issue is concerned with a conflict between the name of a class and the basename of a setter.

@matanlurey
Copy link
Contributor

Ah ok, carry on.

@bwilkerson
Copy link
Member

Created #34289 to track the analyzer work.

@jensjoha jensjoha self-assigned this Sep 4, 2018
@peter-ahe-google
Copy link
Contributor

@eernstg FYI: This is a breaking change from Dart 1. The name of the setter above is "C=" so it didn't use to be an error.

@eernstg
Copy link
Member Author

eernstg commented Sep 5, 2018

Right, older versions of the language specification had a larger set of specific rules for conflicts beyond regular name clashes in the same scope (e.g., among setters and other members, inherited, static, etc), and the current version has been simplified (section 'Class Member Conflicts').

The spec change that I mentioned (9085009) makes the conflicts among class names and members and conflicts among type variable names and members follow the same general approach, and this does indeed introduce a new conflict described in this issue (setter/class-name).

The reason for doing this is that it makes the rules simpler and more consistent.

Are you worried about the breakage? During discussions and reviews about this change we made the assumption that the breakage would be minuscule because setters aren't supposed to have Capitalized names, and it's even more odd-looking to use the class name or the name of a type variable. But you are right, of course, that we should be aware of the actual breakage if it is non-minuscule..

(Edit: Also, I just forgot to mention that we used to get an error message for the setter/class conflict until Feb 2018, which would make it even less likely that we have a lot of breakage.)

@peter-ahe-google
Copy link
Contributor

I'm not worried about the breakage. I'm just pointing out that your statement that this was an error until Feb 6, 2018 isn't correct.

@eernstg
Copy link
Member Author

eernstg commented Sep 5, 2018

Ah, right, the Feb 2018 change in 402e69b fixed a kernel code generation problem, it did not eliminate an incorrect error (which is now a correct error).

But the other justifications for the change should still be valid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
front-end-missing-error legacy-area-front-end Legacy: Use area-dart-model instead. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants