mirror of
https://github.com/dart-lang/sdk
synced 2024-09-16 02:27:39 +00:00
Migration: respect built_value @nullable
annotations even when there is no codegen
Previously, when the migration tool encountered a built_value `@nullable` annotation, it removed it, because when the built_value code generator is consuming a library with null safety enabled, it relies on the presence of a `?` to determine nullability, rather than an annotation. However, there was no logic to actually ensure that the `?` would actually get introduced, because I thought the code generated by built_value would always create the conditions necessary to convince the migration tool to introduce the `?` using its normal graph traversal algorithm. It turns out this is not the case: when `@nullable` appears in an interface class that is used by other built_value classes, but is not itself a built_value class, the migration tool sometimes doesn't have enough information to figure out that it needs to add the `?`. So in this CL, I'm doing what I probably should have done in the first time: adding the necessary logic to the migration tool to ensure that the `@nullable` annotation gets translated into a `?` regardless of whether it is required to by generated code. Bug: https://buganizer.corp.google.com/issues/217863427 Change-Id: I9efe5241634389981a4c56e764bac91b3350c4fb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/233003 Reviewed-by: Samuel Rawlins <srawlins@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
parent
a2f887ea59
commit
5d7445e972
|
@ -242,6 +242,7 @@ enum EdgeOriginKind {
|
|||
alwaysNullableType,
|
||||
angularAnnotation,
|
||||
argumentErrorCheckNotNull,
|
||||
builtValueNullableAnnotation,
|
||||
callTearOff,
|
||||
compoundAssignment,
|
||||
// See [DummyOrigin].
|
||||
|
|
|
@ -29,6 +29,7 @@ import 'package:nnbd_migration/src/edge_origin.dart';
|
|||
import 'package:nnbd_migration/src/expression_checks.dart';
|
||||
import 'package:nnbd_migration/src/nullability_node.dart';
|
||||
import 'package:nnbd_migration/src/nullability_node_target.dart';
|
||||
import 'package:nnbd_migration/src/utilities/built_value_transformer.dart';
|
||||
import 'package:nnbd_migration/src/utilities/completeness_tracker.dart';
|
||||
import 'package:nnbd_migration/src/utilities/hint_utils.dart';
|
||||
import 'package:nnbd_migration/src/utilities/permissive_mode.dart';
|
||||
|
@ -1330,6 +1331,14 @@ class EdgeBuilder extends GeneralizingAstVisitor<DecoratedType>
|
|||
|
||||
@override
|
||||
DecoratedType? visitMethodDeclaration(MethodDeclaration node) {
|
||||
if (BuiltValueTransformer.findNullableAnnotation(node) != null) {
|
||||
_graph.makeNullable(
|
||||
_variables!
|
||||
.decoratedElementType(node.declaredElement!.declaration)
|
||||
.returnType!
|
||||
.node!,
|
||||
BuiltValueNullableOrigin(source, node));
|
||||
}
|
||||
_handleExecutableDeclaration(node, node.declaredElement!, node.metadata,
|
||||
node.returnType, node.parameters, null, node.body, null);
|
||||
_dispatch(node.typeParameters);
|
||||
|
|
|
@ -98,6 +98,17 @@ class ArgumentErrorCheckNotNullOrigin extends EdgeOrigin {
|
|||
EdgeOriginKind get kind => EdgeOriginKind.argumentErrorCheckNotNull;
|
||||
}
|
||||
|
||||
/// Edge origin resulting from a use of built_value's `@nullable` annotation.
|
||||
class BuiltValueNullableOrigin extends EdgeOrigin {
|
||||
BuiltValueNullableOrigin(Source? source, AstNode node) : super(source, node);
|
||||
|
||||
@override
|
||||
String get description => 'method is marked with the `@nullable` annotation';
|
||||
|
||||
@override
|
||||
EdgeOriginKind get kind => EdgeOriginKind.builtValueNullableAnnotation;
|
||||
}
|
||||
|
||||
/// An edge origin used for edges that originated because of a tear-off of
|
||||
/// `call` on a function type.
|
||||
class CallTearOffOrigin extends EdgeOrigin {
|
||||
|
|
|
@ -35,6 +35,7 @@ import 'package:nnbd_migration/src/decorated_type.dart';
|
|||
import 'package:nnbd_migration/src/edit_plan.dart';
|
||||
import 'package:nnbd_migration/src/fix_aggregator.dart';
|
||||
import 'package:nnbd_migration/src/nullability_node.dart';
|
||||
import 'package:nnbd_migration/src/utilities/built_value_transformer.dart';
|
||||
import 'package:nnbd_migration/src/utilities/permissive_mode.dart';
|
||||
import 'package:nnbd_migration/src/utilities/resolution_utils.dart';
|
||||
import 'package:nnbd_migration/src/utilities/where_or_null_transformer.dart';
|
||||
|
@ -1245,25 +1246,13 @@ class _FixBuilderPreVisitor extends GeneralizingAstVisitor<void>
|
|||
|
||||
@override
|
||||
void visitMethodDeclaration(MethodDeclaration node) {
|
||||
if (node.isGetter && node.isAbstract) {
|
||||
for (var annotation in node.metadata) {
|
||||
if (annotation.arguments == null) {
|
||||
var element = annotation.element;
|
||||
if (element is PropertyAccessorElement &&
|
||||
element.name == 'nullable') {
|
||||
if (element.enclosingElement is CompilationUnitElement) {
|
||||
if (element.library.source.uri.toString() ==
|
||||
'package:built_value/built_value.dart') {
|
||||
var info = AtomicEditInfo(
|
||||
NullabilityFixDescription.removeNullableAnnotation, {});
|
||||
(_fixBuilder._getChange(node) as NodeChangeForMethodDeclaration)
|
||||
..annotationToRemove = annotation
|
||||
..removeAnnotationInfo = info;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
var nullableAnnotation = BuiltValueTransformer.findNullableAnnotation(node);
|
||||
if (nullableAnnotation != null) {
|
||||
var info = AtomicEditInfo(
|
||||
NullabilityFixDescription.removeNullableAnnotation, {});
|
||||
(_fixBuilder._getChange(node) as NodeChangeForMethodDeclaration)
|
||||
..annotationToRemove = nullableAnnotation
|
||||
..removeAnnotationInfo = info;
|
||||
}
|
||||
super.visitMethodDeclaration(node);
|
||||
}
|
||||
|
|
|
@ -0,0 +1,28 @@
|
|||
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
import 'package:analyzer/dart/ast/ast.dart';
|
||||
import 'package:analyzer/dart/element/element.dart';
|
||||
|
||||
class BuiltValueTransformer {
|
||||
static Annotation? findNullableAnnotation(MethodDeclaration node) {
|
||||
if (node.isGetter && node.isAbstract) {
|
||||
for (var annotation in node.metadata) {
|
||||
if (annotation.arguments == null) {
|
||||
var element = annotation.element;
|
||||
if (element is PropertyAccessorElement &&
|
||||
element.name == 'nullable') {
|
||||
if (element.enclosingElement is CompilationUnitElement) {
|
||||
if (element.library.source.uri.toString() ==
|
||||
'package:built_value/built_value.dart') {
|
||||
return annotation;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}
|
|
@ -977,6 +977,26 @@ class FooBuilder implements Builder<Foo, FooBuilder> {
|
|||
{path1: file1, path2: file2}, {path1: expected1, path2: anything});
|
||||
}
|
||||
|
||||
Future<void> test_built_value_nullable_getter_interface_only() async {
|
||||
addBuiltValuePackage();
|
||||
var content = '''
|
||||
import 'package:built_value/built_value.dart';
|
||||
|
||||
abstract class Foo {
|
||||
@nullable
|
||||
int get value;
|
||||
}
|
||||
''';
|
||||
var expected = '''
|
||||
import 'package:built_value/built_value.dart';
|
||||
|
||||
abstract class Foo {
|
||||
int? get value;
|
||||
}
|
||||
''';
|
||||
await _checkSingleFileChanges(content, expected);
|
||||
}
|
||||
|
||||
Future<void> test_call_already_migrated_extension() async {
|
||||
var content = '''
|
||||
import 'already_migrated.dart';
|
||||
|
|
Loading…
Reference in a new issue