Skip to content

Commit aaff70f

Browse files
rmconsole2-wfrmconsole2-wf
authored
Merge pull request #303 from Workiva/companion-not-run-on-null-safe-files
FED-2554 Companion codemods should not run on null-safe files
2 parents 86c480a + 6e51ffa commit aaff70f

20 files changed

+687
-9
lines changed

‎.github/workflows/dart_ci.yml‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
strategy:
1616
fail-fast: false
1717
matrix:
18-
sdk: [ 2.18.7, 2.19.6 ]
18+
sdk: [ 2.19.6 ]
1919
steps:
2020
- uses: actions/checkout@v2
2121
- uses: dart-lang/[email protected]
@@ -35,7 +35,7 @@ jobs:
3535

3636
- name: Validate formatting
3737
run: dart run dart_dev format --check
38-
if: always() && steps.install.outcome == 'success' && matrix.sdk == '2.18.7'
38+
if: always() && steps.install.outcome == 'success'
3939

4040
- name: Analyze project source
4141
run: dart analyze
@@ -45,14 +45,14 @@ jobs:
4545
run: |
4646
dart run build_runner build --delete-conflicting-outputs
4747
git diff --exit-code
48-
if: always() && steps.install.outcome == 'success' && matrix.sdk == '2.19.6'
48+
if: always() && steps.install.outcome == 'success'
4949

5050
test:
5151
runs-on: ubuntu-latest
5252
strategy:
5353
fail-fast: false
5454
matrix:
55-
sdk: [ 2.18.7, 2.19.6 ]
55+
sdk: [ 2.19.6 ]
5656
steps:
5757
- uses: actions/checkout@v2
5858
- uses: dart-lang/[email protected]
@@ -74,8 +74,6 @@ jobs:
7474
7575
- name: Create SBOM Release Asset
7676
uses: anchore/sbom-action@v0
77-
# This fails if it runs more than once within a given build
78-
if: matrix.sdk != '2.18.7'
7977
with:
8078
path: ./
8179
format: cyclonedx-json

‎lib/src/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@ class CallbackRefHintSuggestor extends RecursiveAstVisitor<void>
141141
'Could not get resolved result for "${context.relativePath}"');
142142
}
143143
result = r;
144+
// Don't make any updates if the file is already null safe.
145+
if (result.libraryElement.isNonNullableByDefault) {
146+
return;
147+
}
148+
144149
result.unit.visitChildren(this);
145150
}
146151
}

‎lib/src/dart3_suggestors/null_safety_prep/class_component_required_default_props.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ class ClassComponentRequiredDefaultPropsMigrator
8686

8787
@override
8888
Future<void> visitCascadeExpression(CascadeExpression node) async {
89+
// Don't make any updates if the file is already null safe.
90+
if (result.libraryElement.isNonNullableByDefault) {
91+
return;
92+
}
93+
8994
super.visitCascadeExpression(node);
9095

9196
final isDefaultProps = node.ancestors.any((ancestor) {

‎lib/src/dart3_suggestors/null_safety_prep/class_component_required_initial_state.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,11 @@ class ClassComponentRequiredInitialStateMigrator
8080

8181
@override
8282
Future<void> visitCascadeExpression(CascadeExpression node) async {
83+
// Don't make any updates if the file is already null safe.
84+
if (result.libraryElement.isNonNullableByDefault) {
85+
return;
86+
}
87+
8388
super.visitCascadeExpression(node);
8489

8590
final isInitialState = [relevantGetterName, relevantMethodName].contains(

‎lib/src/dart3_suggestors/null_safety_prep/connect_required_props.dart‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ class ConnectRequiredProps extends RecursiveAstVisitor with ClassSuggestor {
6969
throw Exception(
7070
'Could not get resolved result for "${context.relativePath}"');
7171
}
72+
// Don't make any updates if the file is already null safe.
73+
if (result.libraryElement.isNonNullableByDefault) {
74+
return;
75+
}
7276
result.unit.accept(this);
7377

7478
// Add the patches at the end so that all the props to be ignored can be collected

‎lib/src/dart3_suggestors/null_safety_prep/state_mixin_suggestor.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,11 @@ class StateMixinSuggestor extends RecursiveAstVisitor<void>
6262
throw Exception(
6363
'Could not get resolved result for "${context.relativePath}"');
6464
}
65+
66+
// Don't make any updates if the file is already null safe.
67+
if (r.libraryElement.isNonNullableByDefault) {
68+
return;
69+
}
6570
r.unit.accept(this);
6671
}
6772
}

‎lib/src/dart3_suggestors/required_props/codemod/required_props_suggestor.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ class RequiredPropsSuggestor extends RecursiveAstVisitor<void>
4242
throw Exception(
4343
'Could not get resolved result for "${context.relativePath}"');
4444
}
45+
46+
// Don't make any updates if the file is already null safe.
47+
if (result.libraryElement.isNonNullableByDefault) {
48+
return;
49+
}
4550
result.unit.accept(this);
4651
}
4752

‎test/dart3_suggestors/null_safety_prep/callback_ref_hint_suggestor_test.dart‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import 'package:test/test.dart';
1818
import '../../mui_suggestors/components/shared.dart';
1919
import '../../resolved_file_context.dart';
2020
import '../../util.dart';
21+
import '../../util/component_usage_migrator_test.dart';
2122

2223
void main() {
2324
final resolvedContext = SharedAnalysisContext.wsd;
@@ -280,5 +281,64 @@ void main() {
280281
'''),
281282
);
282283
});
284+
285+
group('makes no update if file is already on a null safe Dart version', () {
286+
final resolvedContext = SharedAnalysisContext.overReactNullSafe;
287+
288+
// Warm up analysis in a setUpAll so that if getting the resolved AST times out
289+
// (which is more common for the WSD context), it fails here instead of failing the first test.
290+
setUpAll(resolvedContext.warmUpAnalysis);
291+
292+
late SuggestorTester nullSafeTestSuggestor;
293+
294+
setUp(() {
295+
nullSafeTestSuggestor = getSuggestorTester(
296+
CallbackRefHintSuggestor(),
297+
resolvedContext: resolvedContext,
298+
);
299+
});
300+
301+
test('', () async {
302+
await nullSafeTestSuggestor(
303+
expectedPatchCount: 0,
304+
input: withOverReactImport(/*language=dart*/ '''
305+
import 'dart:html';
306+
307+
content() {
308+
var ref;
309+
(Dom.div()..ref = (ButtonElement r) { ref = r; })();
310+
ref;
311+
}
312+
'''),
313+
);
314+
});
315+
316+
test('unless there is a lang version comment', () async {
317+
await nullSafeTestSuggestor(
318+
input: withOverReactImport(/*language=dart*/ '''
319+
import 'dart:html';
320+
321+
content() {
322+
var ref;
323+
(Dom.div()..ref = (ButtonElement r) { ref = r; })();
324+
ref;
325+
}
326+
''', filePrefix: '// @dart=2.11\n'),
327+
expectedOutput: withOverReactImport(/*language=dart*/ '''
328+
import 'dart:html';
329+
330+
content() {
331+
var ref;
332+
(Dom.div()..ref = (ButtonElement /*?*/ r) { ref = r; })();
333+
ref;
334+
}
335+
''', filePrefix: '// @dart=2.11\n'),
336+
// Ignore error on language version comment.
337+
isExpectedError: (error) =>
338+
error.errorCode.name.toLowerCase() ==
339+
'illegal_language_version_override',
340+
);
341+
});
342+
});
283343
}, tags: 'wsd');
284344
}

‎test/dart3_suggestors/null_safety_prep/class_component_required_default_props_test.dart‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,5 +582,135 @@ void main() {
582582
);
583583
});
584584
});
585+
586+
group('makes no update if file is already on a null safe Dart version', () {
587+
final resolvedContext = SharedAnalysisContext.overReactNullSafe;
588+
589+
// Warm up analysis in a setUpAll so that if getting the resolved AST times out
590+
// (which is more common for the WSD context), it fails here instead of failing the first test.
591+
setUpAll(resolvedContext.warmUpAnalysis);
592+
593+
late SuggestorTester nullSafeTestSuggestor;
594+
595+
setUp(() {
596+
nullSafeTestSuggestor = getSuggestorTester(
597+
ClassComponentRequiredDefaultPropsMigrator(),
598+
resolvedContext: resolvedContext,
599+
);
600+
});
601+
602+
test('', () async {
603+
await nullSafeTestSuggestor(
604+
expectedPatchCount: 0,
605+
input: withOverReactImport(/*language=dart*/ r'''
606+
// ignore: undefined_identifier
607+
UiFactory<FooProps> Foo = castUiFactory(_$Foo);
608+
mixin FooPropsMixin on UiProps {
609+
String? prop1;
610+
late String prop2;
611+
num? prop3;
612+
}
613+
mixin SomeOtherPropsMixin on UiProps {
614+
num? prop4;
615+
}
616+
class FooProps = UiProps with FooPropsMixin, SomeOtherPropsMixin;
617+
class FooComponent extends UiComponent2<FooProps> {
618+
@override
619+
get defaultProps => (newProps()
620+
..prop2 = 'foo'
621+
..prop3 = 1
622+
..prop4 = null
623+
);
624+
625+
@override
626+
render() => null;
627+
}
628+
'''),
629+
);
630+
});
631+
632+
test('unless there is a lang version comment', () async {
633+
await nullSafeTestSuggestor(
634+
input: withOverReactImport(/*language=dart*/ r'''
635+
// ignore: undefined_identifier
636+
UiFactory<FooProps> Foo = castUiFactory(_$Foo);
637+
mixin FooPropsMixin on UiProps {
638+
String notDefaulted;
639+
/// This is a doc comment
640+
/*late*/ String/*!*/ alreadyPatched;
641+
/*late*/ String/*!*/ alreadyPatchedButNoDocComment;
642+
String defaultedNullable;
643+
num defaultedNonNullable;
644+
var untypedDefaultedNonNullable;
645+
var untypedDefaultedNullable;
646+
var untypedNotDefaulted;
647+
}
648+
mixin SomeOtherPropsMixin on UiProps {
649+
num anotherDefaultedNonNullable;
650+
Function defaultedNonNullableFn;
651+
List<num> defaultedNonNullableList;
652+
}
653+
class FooProps = UiProps with FooPropsMixin, SomeOtherPropsMixin;
654+
class FooComponent extends UiComponent2<FooProps> {
655+
@override
656+
get defaultProps => (newProps()
657+
..alreadyPatched = 'foo'
658+
..untypedDefaultedNonNullable = 1
659+
..untypedDefaultedNullable = null
660+
..defaultedNullable = null
661+
..defaultedNonNullable = 2.1
662+
..anotherDefaultedNonNullable = 1.1
663+
..defaultedNonNullableFn = () {}
664+
..defaultedNonNullableList = []
665+
);
666+
667+
@override
668+
render() => null;
669+
}
670+
''', filePrefix: '// @dart=2.11\n'),
671+
expectedOutput: withOverReactImport(/*language=dart*/ r'''
672+
// ignore: undefined_identifier
673+
UiFactory<FooProps> Foo = castUiFactory(_$Foo);
674+
mixin FooPropsMixin on UiProps {
675+
String notDefaulted;
676+
/// This is a doc comment
677+
/*late*/ String/*!*/ alreadyPatched;
678+
/*late*/ String/*!*/ alreadyPatchedButNoDocComment;
679+
/*late*/ String/*?*/ defaultedNullable;
680+
/*late*/ num/*!*/ defaultedNonNullable;
681+
/*late*/ dynamic/*!*/ untypedDefaultedNonNullable;
682+
/*late*/ dynamic/*?*/ untypedDefaultedNullable;
683+
var untypedNotDefaulted;
684+
}
685+
mixin SomeOtherPropsMixin on UiProps {
686+
/*late*/ num/*!*/ anotherDefaultedNonNullable;
687+
/*late*/ Function/*!*/ defaultedNonNullableFn;
688+
/*late*/ List<num>/*!*/ defaultedNonNullableList;
689+
}
690+
class FooProps = UiProps with FooPropsMixin, SomeOtherPropsMixin;
691+
class FooComponent extends UiComponent2<FooProps> {
692+
@override
693+
get defaultProps => (newProps()
694+
..alreadyPatched = 'foo'
695+
..untypedDefaultedNonNullable = 1
696+
..untypedDefaultedNullable = null
697+
..defaultedNullable = null
698+
..defaultedNonNullable = 2.1
699+
..anotherDefaultedNonNullable = 1.1
700+
..defaultedNonNullableFn = () {}
701+
..defaultedNonNullableList = []
702+
);
703+
704+
@override
705+
render() => null;
706+
}
707+
''', filePrefix: '// @dart=2.11\n'),
708+
// Ignore error on language version comment.
709+
isExpectedError: (error) =>
710+
error.errorCode.name.toLowerCase() ==
711+
'illegal_language_version_override',
712+
);
713+
});
714+
});
585715
});
586716
}

0 commit comments

Comments
 (0)