Skip to content

Commit

Permalink
Merge pull request #948 from Workiva/fix-required-prop-null-safe-cond…
Browse files Browse the repository at this point in the history
…itional

FED-3159 Fix required prop null-safe-conditional linting
  • Loading branch information
rmconsole2-wf authored Sep 20, 2024
2 parents 398f08c + f0e5243 commit 0705616
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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');
Expand Down
3 changes: 1 addition & 2 deletions tools/analyzer_plugin/lib/src/diagnostic/invalid_child.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -220,7 +219,8 @@ class MissingRequiredPropDiagnostic extends ComponentUsageDiagnosticContributor
continue;
}

if (withNullability(result) || 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),
Expand Down
6 changes: 0 additions & 6 deletions tools/analyzer_plugin/lib/src/util/null_safety_utils.dart

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<void> test_noErrorsWhenNoRequiredProps() async {
await expectNoErrors(newSourceWithPrefix(/*language=dart*/ r'''
Expand Down Expand Up @@ -138,16 +140,26 @@ class MissingRequiredPropTest_NoErrors extends MissingRequiredPropTest {
test() => InheritsLateRequired()();
'''));
}

Future<void> test_missingLateRequiredDeclaredInOtherLib() async {
final source = newSourceWithPrefix(/*language=dart*/ r'''
test() => ReduxProvider()();
''');
final selection = createSelection(source, '#ReduxProvider()#');
final allErrors = await getAllErrors(source);
expect(
allErrors,
unorderedEquals(<dynamic>[
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'''
Expand Down

0 comments on commit 0705616

Please sign in to comment.