From bcbe3a54ff3697efd104a68a1269bae855bfc7d9 Mon Sep 17 00:00:00 2001 From: Keerti Parthasarathy Date: Thu, 20 Jun 2024 18:10:32 +0000 Subject: [PATCH] Fix bug in AddDiagnosticReference when using it via CLI. Fixes https://github.com/dart-lang/sdk/issues/55772. Change-Id: Ic77f67486f71bdfea726ea3161d83728326b2c5b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/372120 Commit-Queue: Keerti Parthasarathy Reviewed-by: Brian Wilkerson --- .../add_diagnostic_property_reference.dart | 236 ++++++++++++++++++ ...dd_diagnostic_property_reference_test.dart | 87 ++++++- 2 files changed, 319 insertions(+), 4 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/add_diagnostic_property_reference.dart b/pkg/analysis_server/lib/src/services/correction/dart/add_diagnostic_property_reference.dart index f8542daec83..4229ad9b0b5 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/add_diagnostic_property_reference.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/add_diagnostic_property_reference.dart @@ -4,11 +4,14 @@ import 'package:analysis_server/src/services/correction/assist.dart'; import 'package:analysis_server/src/services/correction/fix.dart'; +import 'package:analysis_server/src/services/linter/lint_names.dart'; import 'package:analysis_server/src/utilities/extensions/flutter.dart'; import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; import 'package:analyzer/dart/element/type.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:analyzer/src/dart/ast/utilities.dart'; import 'package:analyzer_plugin/utilities/assist/assist.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart'; @@ -54,6 +57,11 @@ class AddDiagnosticPropertyReference extends ResolvedCorrectionProducer { return; } + if (applyingBulkFixes) { + await _fixAllDiagnosticPropertyReferences(builder, classDeclaration); + return; + } + var type = _getReturnType(node); if (type == null) { return; @@ -191,6 +199,222 @@ class AddDiagnosticPropertyReference extends ResolvedCorrectionProducer { } } + /// Fixes all instances of the [LintNames.diagnostic_describe_all_properties] in the given + /// [declaration]. + Future _fixAllDiagnosticPropertyReferences( + ChangeBuilder builder, ClassDeclaration declaration) async { + var propertyErrors = _getAllDiagnosticsInClass(declaration); + + // Create fixes only when its the first error. + if (propertyErrors.isNotEmpty && + errorOffset != propertyErrors.first.offset) { + return; + } + + void writePropertyReference(DartEditBuilder builder, + {required String prefix, + required String builderName, + required _PropertyInfo property}) { + builder.write('$prefix$builderName.add(${property.constructorId}'); + var type = property.type; + if (property.typeArgs != null) { + builder.write('<'); + builder.writeTypes(property.typeArgs); + builder.write('>'); + } else if (type is DynamicType || type is InvalidType) { + var declType = property.declType; + + if (declType != null) { + var typeText = utils.getNodeText(declType); + if (typeText != 'dynamic') { + builder.write('<'); + builder.write(utils.getNodeText(declType)); + builder.write('>'); + } + } + } + builder.writeln( + "${property.constructorName}('${property.name}', ${property.name}));"); + } + + var properties = <_PropertyInfo>[]; + + // Compute the information for all the properties to be added. + for (var error in propertyErrors) { + var selectionOffset = error.offset; + var selectionEnd = selectionOffset + error.length; + var locator = NodeLocator(selectionOffset, selectionEnd); + var node = locator.searchWithin(unitResult.unit); + if (node == null) { + continue; + } + var propertyInfo = _getPropertyInfo(node); + if (propertyInfo.type != null) { + properties.add(propertyInfo); + } + } + + if (properties.isEmpty) { + return; + } + + var debugFillProperties = declaration.members + .whereType() + .where((e) => e.name.lexeme == 'debugFillProperties') + .singleOrNull; + + if (debugFillProperties == null) { + await builder.addDartFileEdit(file, (builder) { + builder.insertMethod(declaration, (builder) { + var declPrefix = utils.oneIndent; + var bodyPrefix = utils.twoIndents; + + builder.writeln('@override'); + builder.writeln( + '${declPrefix}void debugFillProperties(DiagnosticPropertiesBuilder properties) {'); + builder + .writeln('${bodyPrefix}super.debugFillProperties(properties);'); + + for (var property in properties) { + writePropertyReference(builder, + prefix: bodyPrefix, + builderName: 'properties', + property: property); + } + builder.write('$declPrefix}'); + }); + }); + return; + } + + var body = debugFillProperties.body; + if (body is BlockFunctionBody) { + var functionBody = body; + + int offset; + String prefix; + if (functionBody.block.statements.isEmpty) { + offset = functionBody.block.leftBracket.offset; + prefix = utils.getLinePrefix(offset) + utils.oneIndent; + } else { + offset = functionBody.block.statements.last.endToken.offset; + prefix = utils.getLinePrefix(offset); + } + + var parameterList = debugFillProperties.parameters; + if (parameterList == null) { + return; + } + + String? propertiesBuilderName; + for (var parameter in parameterList.parameters) { + if (parameter is SimpleFormalParameter) { + var type = parameter.type; + var identifier = parameter.name; + if (type is NamedType && identifier != null) { + if (type.name2.lexeme == 'DiagnosticPropertiesBuilder') { + propertiesBuilderName = identifier.lexeme; + break; + } + } + } + } + if (propertiesBuilderName == null) { + return; + } + + var final_propertiesBuilderName = propertiesBuilderName; + await builder.addDartFileEdit(file, (builder) { + builder.addInsertion(utils.getLineNext(offset), (builder) { + for (var property in properties) { + writePropertyReference(builder, + prefix: prefix, + builderName: final_propertiesBuilderName, + property: property); + } + }); + }); + } + } + + /// Returns a list of all the [AnalysisError]'s of type + /// [LintNames.diagnostic_describe_all_properties] fpr the given [declaration]. + List _getAllDiagnosticsInClass(ClassDeclaration declaration) { + var propertyErrors = []; + var startOffset = declaration.offset; + var endOffset = startOffset + declaration.length; + for (var error in unitResult.errors) { + var errorCode = error.errorCode; + if (errorCode.type == ErrorType.LINT && + errorCode.name == LintNames.diagnostic_describe_all_properties && + error.offset > startOffset && + error.offset < endOffset) { + propertyErrors.add(error); + } + } + + return propertyErrors; + } + + /// Computes the information for the proerty at the given [node]. + _PropertyInfo _getPropertyInfo(AstNode node) { + String? name; + if (node is MethodDeclaration) { + name = node.name.lexeme; + } else if (node is VariableDeclaration) { + name = node.name.lexeme; + } + var type = _getReturnType(node); + if (type == null) { + return _PropertyInfo(name, type, '', [], '', null); + } + + String constructorId; + List? typeArgs; + var constructorName = ''; + + if (type is FunctionType) { + constructorId = 'ObjectFlagProperty'; + typeArgs = [type]; + constructorName = '.has'; + } else if (type.isDartCoreInt) { + constructorId = 'IntProperty'; + } else if (type.isDartCoreDouble) { + constructorId = 'DoubleProperty'; + } else if (type.isDartCoreString) { + constructorId = 'StringProperty'; + } else if (_isEnum(type)) { + constructorId = 'EnumProperty'; + typeArgs = [type]; + } else if (_isIterable(type)) { + constructorId = 'IterableProperty'; + typeArgs = (type as InterfaceType).typeArguments; + } else if (type.isColor) { + constructorId = 'ColorProperty'; + } else if (type.isMatrix4) { + constructorId = 'TransformProperty'; + } else { + constructorId = 'DiagnosticsProperty'; + if (!(type is DynamicType || type is InvalidType)) { + typeArgs = [type]; + } + } + + TypeAnnotation? declType; + if (type is DynamicType || type is InvalidType) { + var decl = node.thisOrAncestorOfType(); + if (decl != null) { + declType = decl.type; + // getter + } else if (node is MethodDeclaration) { + declType = node.returnType; + } + } + + return _PropertyInfo( + name, type, constructorId, typeArgs, constructorName, declType); + } + /// Return the return type of the given [node]. DartType? _getReturnType(AstNode node) { if (node is MethodDeclaration) { @@ -217,3 +441,15 @@ class AddDiagnosticPropertyReference extends ResolvedCorrectionProducer { return type.asInstanceOf(typeProvider.iterableElement) != null; } } + +class _PropertyInfo { + final String? name; + final DartType? type; + final String constructorId; + final List? typeArgs; + final String constructorName; + final TypeAnnotation? declType; + + _PropertyInfo(this.name, this.type, this.constructorId, this.typeArgs, + this.constructorName, this.declType); +} diff --git a/pkg/analysis_server/test/src/services/correction/fix/add_diagnostic_property_reference_test.dart b/pkg/analysis_server/test/src/services/correction/fix/add_diagnostic_property_reference_test.dart index b87d6c5fffe..14cb59e36e1 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/add_diagnostic_property_reference_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/add_diagnostic_property_reference_test.dart @@ -22,6 +22,88 @@ class AddDiagnosticPropertyReferenceBulkTest extends BulkFixProcessorTest { @override String get lintCode => LintNames.diagnostic_describe_all_properties; + @override + void setUp() { + super.setUp(); + writeTestPackageConfig( + flutter: true, + ); + } + + Future test_multiple_no_debugFillPropertiesMethod() async { + createAnalysisOptionsFile( + lints: [LintNames.diagnostic_describe_all_properties]); + await resolveTestCode(r''' +import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart'; + +class C extends Widget with Diagnosticable { + bool get absorbing => _absorbing; + bool _absorbing = false; + String logBuffer = ''; +} +'''); + + await assertHasFix(r''' +import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart'; + +class C extends Widget with Diagnosticable { + bool get absorbing => _absorbing; + bool _absorbing = false; + String logBuffer = ''; + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(DiagnosticsProperty('absorbing', absorbing)); + properties.add(StringProperty('logBuffer', logBuffer)); + } +} +'''); + } + + Future test_multiple_with_debugFillPropertiesMethod() async { + createAnalysisOptionsFile( + lints: [LintNames.diagnostic_describe_all_properties]); + await resolveTestCode(r''' +import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart'; + +class C extends Widget with Diagnosticable { + bool get absorbing => _absorbing; + bool _absorbing = false; + String logBuffer = ''; + int field = 0; + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + } +} +'''); + + await assertHasFix(r''' +import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart'; + +class C extends Widget with Diagnosticable { + bool get absorbing => _absorbing; + bool _absorbing = false; + String logBuffer = ''; + int field = 0; + + @override + void debugFillProperties(DiagnosticPropertiesBuilder properties) { + super.debugFillProperties(properties); + properties.add(DiagnosticsProperty('absorbing', absorbing)); + properties.add(StringProperty('logBuffer', logBuffer)); + properties.add(IntProperty('field', field)); + } +} +'''); + } + Future test_singleFile() async { writeTestPackageConfig(flutter: true); await resolveTestCode(''' @@ -388,10 +470,6 @@ import 'package:flutter/widgets.dart'; class C extends Widget with Diagnosticable { Iterable field = []; - @override - void debugFillProperties(DiagnosticPropertiesBuilder properties) { - super.debugFillProperties(properties); - } } '''); await assertHasFix(''' @@ -400,6 +478,7 @@ import 'package:flutter/widgets.dart'; class C extends Widget with Diagnosticable { Iterable field = []; + @override void debugFillProperties(DiagnosticPropertiesBuilder properties) { super.debugFillProperties(properties);