From 5736c705e5c9675cee29214e0310887428d091ab Mon Sep 17 00:00:00 2001 From: Greg Littlefield Date: Wed, 18 Sep 2024 15:31:47 -0700 Subject: [PATCH 1/3] Replace utility with built-in getter --- .../analyzer_plugin/lib/src/assist/refs/add_create_ref.dart | 3 +-- tools/analyzer_plugin/lib/src/diagnostic/invalid_child.dart | 3 +-- .../lib/src/diagnostic/missing_required_prop.dart | 3 +-- tools/analyzer_plugin/lib/src/util/null_safety_utils.dart | 6 ------ 4 files changed, 3 insertions(+), 12 deletions(-) delete mode 100644 tools/analyzer_plugin/lib/src/util/null_safety_utils.dart diff --git a/tools/analyzer_plugin/lib/src/assist/refs/add_create_ref.dart b/tools/analyzer_plugin/lib/src/assist/refs/add_create_ref.dart index 9bc4767bf..6e9b3f847 100644 --- a/tools/analyzer_plugin/lib/src/assist/refs/add_create_ref.dart +++ b/tools/analyzer_plugin/lib/src/assist/refs/add_create_ref.dart @@ -11,7 +11,6 @@ import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart'; import 'package:over_react_analyzer_plugin/src/indent_util.dart'; import 'package:over_react_analyzer_plugin/src/util/ast_util.dart'; import 'package:over_react_analyzer_plugin/src/util/function_components.dart'; -import 'package:over_react_analyzer_plugin/src/util/null_safety_utils.dart' as utils; import 'package:over_react_analyzer_plugin/src/util/util.dart'; enum RefTypeToReplace { @@ -49,7 +48,7 @@ void addUseOrCreateRef( RefTypeToReplace? refTypeToReplace; Expression? callbackRefPropRhs; - final withNullability = utils.withNullability(result); + final withNullability = result.libraryElement.isNonNullableByDefault; final refTypeName = usage.isDom ? 'Element${withNullability ? '?' : ''}' : 'dynamic'; final refProp = usage.cascadedProps.firstWhereOrNull((prop) => prop.name.name == 'ref'); diff --git a/tools/analyzer_plugin/lib/src/diagnostic/invalid_child.dart b/tools/analyzer_plugin/lib/src/diagnostic/invalid_child.dart index 5190752a1..487a4622f 100644 --- a/tools/analyzer_plugin/lib/src/diagnostic/invalid_child.dart +++ b/tools/analyzer_plugin/lib/src/diagnostic/invalid_child.dart @@ -6,7 +6,6 @@ import 'package:analyzer/dart/element/type_system.dart'; import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart'; import 'package:over_react_analyzer_plugin/src/util/ast_util.dart'; import 'package:over_react_analyzer_plugin/src/util/constants.dart'; -import 'package:over_react_analyzer_plugin/src/util/null_safety_utils.dart' as utils; import 'package:over_react_analyzer_plugin/src/util/react_types.dart'; import 'package:over_react_analyzer_plugin/src/fluent_interface_util.dart'; import 'package:over_react_analyzer_plugin/src/util/util.dart'; @@ -71,7 +70,7 @@ class InvalidChildDiagnostic extends ComponentUsageDiagnosticContributor { await validateReactChildType(argument.staticType, result.typeSystem, result.typeProvider, onInvalidType: (invalidType) async { final location = result.locationFor(argument); - final withNullability = utils.withNullability(result); + final withNullability = result.libraryElement.isNonNullableByDefault; if (couldBeMissingBuilderInvocation(argument)) { await collector.addErrorWithFix( diff --git a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart index 8f249fa2b..1d160aec5 100644 --- a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart +++ b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart @@ -4,7 +4,6 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:over_react_analyzer_plugin/src/diagnostic/analyzer_debug_helper.dart'; import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart'; import 'package:over_react_analyzer_plugin/src/util/ast_util.dart'; -import 'package:over_react_analyzer_plugin/src/util/null_safety_utils.dart'; import 'package:over_react_analyzer_plugin/src/util/pretty_print.dart'; import 'package:over_react_analyzer_plugin/src/util/prop_declarations/props_set_by_factory.dart'; import 'package:over_react_analyzer_plugin/src/util/prop_forwarding/forwarded_props.dart'; @@ -220,7 +219,7 @@ class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor continue; } - if (withNullability(result) || requiredness != PropRequiredness.late) { + if (result.libraryElement.isNonNullableByDefault || requiredness != PropRequiredness.late) { await collector.addErrorWithFix( _codeForRequiredness(requiredness), result.locationFor(usage.builder), diff --git a/tools/analyzer_plugin/lib/src/util/null_safety_utils.dart b/tools/analyzer_plugin/lib/src/util/null_safety_utils.dart deleted file mode 100644 index 9c34564c6..000000000 --- a/tools/analyzer_plugin/lib/src/util/null_safety_utils.dart +++ /dev/null @@ -1,6 +0,0 @@ -import 'package:analyzer/dart/analysis/features.dart'; -import 'package:analyzer/dart/analysis/results.dart'; - -/// Returns whether [result] is in a library with null safety enabled. -bool withNullability(ResolvedUnitResult result) => - result.unit.declaredElement?.library.featureSet.isEnabled(Feature.non_nullable) ?? false; From 4e526378355bb1e40b2c7f3f7b8c6106d2f8e859 Mon Sep 17 00:00:00 2001 From: Greg Littlefield Date: Wed, 18 Sep 2024 15:43:22 -0700 Subject: [PATCH 2/3] Add regression test --- .../missing_required_prop_test.dart | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart b/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart index 98fa0c87f..26963b08e 100644 --- a/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart +++ b/tools/analyzer_plugin/test/integration/diagnostics/non_null_safe/missing_required_prop_test.dart @@ -24,20 +24,22 @@ import '../../test_bases/diagnostic_test_base.dart'; void main() { defineReflectiveSuite(() { - defineReflectiveTests(MissingRequiredPropTest_NoErrors); + defineReflectiveTests(MissingRequiredPropTest); defineReflectiveTests(MissingRequiredPropTest_MissingAnnotationRequired); }); } -abstract class MissingRequiredPropTest extends DiagnosticTestBase { +abstract class MissingRequiredPropTestBase extends DiagnosticTestBase { @override - get fixKindUnderTest => null; + get fixKindUnderTest => MissingRequiredPropDiagnostic.fixKind; Source newSourceWithPrefix(String sourceFragment) => newSource(sourcePrefix + sourceFragment); static const sourcePrefix = /*language=dart*/ r''' // @dart=2.11 import 'package:over_react/over_react.dart'; +// ignore: unused_import +import 'package:over_react/over_react_redux.dart'; part '{{FILE_BASENAME_WITHOUT_EXTENSION}}.over_react.g.dart'; @@ -68,9 +70,9 @@ mixin WithAnnotationRequiredProps on UiProps { } @reflectiveTest -class MissingRequiredPropTest_NoErrors extends MissingRequiredPropTest { +class MissingRequiredPropTest extends MissingRequiredPropTestBase { @override - get errorUnderTest => null; + get errorUnderTest => MissingRequiredPropDiagnostic.lateRequiredCode; Future test_noErrorsWhenNoRequiredProps() async { await expectNoErrors(newSourceWithPrefix(/*language=dart*/ r''' @@ -138,16 +140,26 @@ class MissingRequiredPropTest_NoErrors extends MissingRequiredPropTest { test() => InheritsLateRequired()(); ''')); } + + Future test_missingLateRequiredDeclaredInOtherLib() async { + final source = newSourceWithPrefix(/*language=dart*/ r''' + test() => ReduxProvider()(); + '''); + final selection = createSelection(source, '#ReduxProvider()#'); + final allErrors = await getAllErrors(source); + expect( + allErrors, + unorderedEquals([ + isAnErrorUnderTest(locatedAt: selection).havingMessage(contains("'store' from 'ReduxProviderPropsMixin'")), + ])); + } } @reflectiveTest -class MissingRequiredPropTest_MissingAnnotationRequired extends MissingRequiredPropTest { +class MissingRequiredPropTest_MissingAnnotationRequired extends MissingRequiredPropTestBase { @override get errorUnderTest => MissingRequiredPropDiagnostic.annotationRequiredCode; - @override - get fixKindUnderTest => MissingRequiredPropDiagnostic.fixKind; - // This lint is disabled by default, so we have to opt into it. @override String get analysisOptionsYamlContents => r''' From f0e5243f38643edce943514c10a8198cf7c62f64 Mon Sep 17 00:00:00 2001 From: Greg Littlefield Date: Thu, 19 Sep 2024 11:32:16 -0700 Subject: [PATCH 3/3] Fix check for conditional missing required prop linting --- .../lib/src/diagnostic/missing_required_prop.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart index 1d160aec5..4eaf29008 100644 --- a/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart +++ b/tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart @@ -219,7 +219,8 @@ class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor continue; } - if (result.libraryElement.isNonNullableByDefault || requiredness != PropRequiredness.late) { + // Late required prop validation is only enabled for props classes that have migrated to null safety. + if (propsClassElement.library.isNonNullableByDefault || requiredness != PropRequiredness.late) { await collector.addErrorWithFix( _codeForRequiredness(requiredness), result.locationFor(usage.builder),