Fix constructor references in assignments

I maintain the choice to _not_ rewrite a PrefixedIdentifier to a
ConstructorReference in the case that it is the left side of an
AssignmentExpression, because doing so requires extra additional
checking during assignment verification, and probably a new error code
saying a constructor cannot be assigned to, similar to the error code
saying that a method cannot be assigned to. This is all feasible, but
I think it is very reasonable, in AstRewrite, to say, "This prefixed
identifier does seem to refer to a constructor, but it's the left
side of an assignment, so I'm not going to confidently do a rewrite;
instead I will leave this alone and let the assignment verification
code handle it."

Bug: b/200948019
Change-Id: I90c6a157c2992d4935acfb593f38518f0388bced
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214320
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This commit is contained in:
Sam Rawlins 2021-09-23 19:38:49 +00:00 committed by commit-bot@chromium.org
parent e7ce24744b
commit 032738141e
2 changed files with 56 additions and 5 deletions

View file

@ -225,24 +225,27 @@ class AstRewriter {
/// The [PrefixedIdentifier] may need to be rewritten as a
/// [ConstructorReference].
AstNode prefixedIdentifier(Scope nameScope, PrefixedIdentifier node) {
if (node.parent is Annotation) {
var parent = node.parent;
if (parent is Annotation) {
// An annotations which is a const constructor invocation can initially be
// represented with a [PrefixedIdentifier]. Do not rewrite such nodes.
return node;
}
if (node.parent is CommentReference) {
if (parent is CommentReference) {
// TODO(srawlins): This probably should be rewritten to a
// [ConstructorReference] at some point.
return node;
}
if (parent is AssignmentExpression && parent.leftHandSide == node) {
// A constructor cannot be assigned to, in some expression like
// `C.new = foo`; do not rewrite.
return node;
}
var identifier = node.identifier;
if (identifier.isSynthetic) {
// This isn't a constructor reference.
return node;
}
if (node.parent is AssignmentExpression) {
return node;
}
var prefix = node.prefix;
var element = nameScope.lookup(prefix.name).getter;
if (element is ClassElement) {

View file

@ -14,6 +14,14 @@ import 'context_collection_resolution.dart';
main() {
defineReflectiveSuite(() {
defineReflectiveTests(AstRewriteMethodInvocationTest);
defineReflectiveTests(AstRewritePrefixedIdentifierTest);
// TODO(srawlins): Add AstRewriteInstanceCreationExpressionTest test, likely
// moving many test cases from ConstructorReferenceResolutionTest,
// FunctionReferenceResolutionTest, and TypeLiteralResolutionTest.
// TODO(srawlins): Add AstRewritePropertyAccessTest test, likely
// moving many test cases from ConstructorReferenceResolutionTest,
// FunctionReferenceResolutionTest, and TypeLiteralResolutionTest.
});
}
@ -484,3 +492,43 @@ void f() {
expect(argumentStrings, expectedArguments);
}
}
@reflectiveTest
class AstRewritePrefixedIdentifierTest extends PubPackageResolutionTest {
test_constructorReference_inAssignment_onLeftSide() async {
await assertErrorsInCode('''
class C {}
void f() {
C.new = 1;
}
''', [
error(CompileTimeErrorCode.UNDEFINED_SETTER, 27, 3),
]);
var identifier = findNode.prefixed('C.new');
// The left side of the assignment is resolved by
// [PropertyElementResolver._resolveTargetClassElement], which looks for
// getters and setters on `C`, and does not recover with other elements
// (methods, constructors). This prefixed identifier can have a real
// `staticElement` if we add such recovery.
assertElement(identifier, null);
}
test_constructorReference_inAssignment_onRightSide() async {
await assertNoErrorsInCode('''
class C {}
Function? f;
void g() {
f = C.new;
}
''');
var identifier = findNode.constructorReference('C.new');
assertElement(identifier, findElement.unnamedConstructor('C'));
}
// TODO(srawlins): Complete tests of all cases of rewriting (or not) a
// prefixed identifier.
}