diff --git a/lib/src/builder/builder.dart b/lib/src/builder/builder.dart index 07a18cea4..9e835752d 100644 --- a/lib/src/builder/builder.dart +++ b/lib/src/builder/builder.dart @@ -121,10 +121,7 @@ class OverReactBuilder extends Builder { } // Collect all of the part files for this library. - final parts = libraryUnit.directives - .whereType() - // Ignore all generated `.g.dart` parts. - .where((part) => !part.uri.stringValue.endsWith('.g.dart')); + final parts = getNonGeneratedParts(libraryUnit); // Generate over_react code for the input library. generateForFile(source, buildStep.inputId, libraryUnit); diff --git a/lib/src/builder/util.dart b/lib/src/builder/util.dart index e7efb8d31..31703229e 100644 --- a/lib/src/builder/util.dart +++ b/lib/src/builder/util.dart @@ -42,6 +42,12 @@ Uri idToPackageUri(AssetId id) { path: p.url.join(id.package, id.path.replaceFirst('lib/', ''))); } +Iterable getNonGeneratedParts(CompilationUnit libraryUnit) { + return libraryUnit.directives + .whereType() + // Ignore all generated `.g.dart` parts. + .where((part) => !part.uri.stringValue.endsWith('.g.dart')); +} /// Returns true if the given compilation unit is a part file. bool isPart(CompilationUnit unit) => unit.directives.any((directive) => directive is PartOfDirective); diff --git a/tools/analyzer_plugin/lib/src/diagnostic/boilerplate_validator.dart b/tools/analyzer_plugin/lib/src/diagnostic/boilerplate_validator.dart index c5be0cca4..8221d93db 100644 --- a/tools/analyzer_plugin/lib/src/diagnostic/boilerplate_validator.dart +++ b/tools/analyzer_plugin/lib/src/diagnostic/boilerplate_validator.dart @@ -7,6 +7,8 @@ import 'package:analyzer_plugin/protocol/protocol_common.dart'; import 'package:meta/meta.dart'; // ignore: implementation_imports import 'package:over_react/src/builder/parsing.dart'; +// ignore: implementation_imports +import 'package:over_react/src/builder/util.dart'; import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart'; import 'package:over_react_analyzer_plugin/src/util/boilerplate_utils.dart'; import 'package:source_span/source_span.dart'; @@ -31,12 +33,30 @@ class BoilerplateValidatorDiagnostic extends DiagnosticContributor { PartDirective _overReactGeneratedPartDirective; bool _overReactGeneratedPartDirectiveIsValid; - @override - Future computeErrors(result, collector) async { - _overReactGeneratedPartDirective = getOverReactGeneratedPartDirective(result.unit); - _overReactGeneratedPartDirectiveIsValid = _overReactGeneratedPartDirective != null && - overReactGeneratedPartDirectiveIsValid(_overReactGeneratedPartDirective, result.uri); + /// Returns true if the [unit] representing a part file has declarations. + /// + /// Does not report any errors for the part file, as those are handled when the part file is analyzed + bool _partHasDeclarations(CompilationUnit unit, ResolvedUnitResult parentResult) { + return getBoilerplateDeclarations( + detectBoilerplateMembers(unit), + ErrorCollector.callback( + SourceFile.fromString(parentResult.content, url: parentResult.path), + onError: (message, [span]) { + // no-op. It is assumed this method will run for parent files, and the part file will get analyzed in it's own + // context + }, + onWarning: (message, [span]) { + // no-op. It is assumed this method will run for parent files, and the part file will get analyzed in it's own + // context + }, + )).isNotEmpty; + } + /// Computes errors for over_react boilerplate + /// + /// Also returns whether the component has valid over_react declarations, which is useful in determining whether to + /// validate the generated part directive. + Future _computeBoilerplateErrors(ResolvedUnitResult result, DiagnosticCollector collector) async { final debugMatch = _debugFlagPattern.firstMatch(result.content); final debug = debugMatch != null; if (debug) { @@ -67,6 +87,8 @@ class BoilerplateValidatorDiagnostic extends DiagnosticContributor { // Do not lint anything that is not a likely boilerplate member that will actually get generated. if (member.versionConfidences.toList().every((vcp) => vcp.confidence <= Confidence.neutral)) continue; + if (isPart(result.unit)) continue; + if (_overReactGeneratedPartDirective == null) { await _addPartDirectiveErrorForMember( result: result, @@ -100,10 +122,12 @@ class BoilerplateValidatorDiagnostic extends DiagnosticContributor { } } } + return declarations.isNotEmpty; + } - // TODO: this logic does not handle when the component is already in a part file. We should refactor this to share logic that the builder uses to validate parts. - // - if (declarations.isEmpty && _overReactGeneratedPartDirective != null) { + Future _computePartDirectiveErrors( + ResolvedUnitResult result, DiagnosticCollector collector, bool hasDeclarations) async { + if (!hasDeclarations && _overReactGeneratedPartDirective != null) { await collector.addErrorWithFix( errorCode, result.locationFor(_overReactGeneratedPartDirective), @@ -118,13 +142,15 @@ class BoilerplateValidatorDiagnostic extends DiagnosticContributor { removeOverReactGeneratedPartDirective(builder, result.unit); }), ); - } else if (declarations.isNotEmpty && + } else if (hasDeclarations && _overReactGeneratedPartDirective != null && !_overReactGeneratedPartDirectiveIsValid) { await collector.addErrorWithFix( errorCode, result.locationFor(_overReactGeneratedPartDirective), - errorMessageArgs: ['The generated part name must match the name of the file that contains it.'], + errorMessageArgs: [ + 'The generated part name must match the name of the file that contains it, but with `over_react.g.dart` for the file extension.' + ], fixKind: invalidGeneratedPartFixKind, computeFix: () => buildFileEdit(result, (builder) { fixOverReactGeneratedPartDirective(builder, result.unit, result.uri); @@ -133,6 +159,35 @@ class BoilerplateValidatorDiagnostic extends DiagnosticContributor { } } + @override + Future computeErrors(result, collector) async { + _overReactGeneratedPartDirective = getOverReactGeneratedPartDirective(result.unit); + _overReactGeneratedPartDirectiveIsValid = _overReactGeneratedPartDirective != null && + overReactGeneratedPartDirectiveIsValid(_overReactGeneratedPartDirective, result.uri); + + final hasDeclarations = await _computeBoilerplateErrors(result, collector); + if (isPart(result.unit)) { + return; + } + + final parts = getNonGeneratedParts(result.unit); + + // compute errors for parts files + var anyPartHasDeclarations = false; + for (final part in parts) { + final uri = part.uriSource?.uri; + // URI could not be resolved or source does not exist + if (uri == null) continue; + final partResult = result.session.getParsedUnit(result.session.uriConverter.uriToPath(uri)); + + if (_partHasDeclarations(partResult.unit, result)) { + anyPartHasDeclarations = true; + } + } + + await _computePartDirectiveErrors(result, collector, hasDeclarations || anyPartHasDeclarations); + } + Future _addPartDirectiveErrorForMember({ @required ResolvedUnitResult result, @required DiagnosticCollector collector, @@ -177,3 +232,11 @@ extension on Iterable { }); } } + +// TODO use the version from over_react instead after initial release +Iterable getNonGeneratedParts(CompilationUnit libraryUnit) { + return libraryUnit.directives + .whereType() + // Ignore all generated `.g.dart` parts. + .where((part) => !part.uri.stringValue.endsWith('.g.dart')); +} diff --git a/tools/analyzer_plugin/playground/web/invalid_generated_part_filename_parent.dart b/tools/analyzer_plugin/playground/web/invalid_generated_part_filename_parent.dart new file mode 100644 index 000000000..f7629dbdd --- /dev/null +++ b/tools/analyzer_plugin/playground/web/invalid_generated_part_filename_parent.dart @@ -0,0 +1,6 @@ +library invalid_generated_part_filename; + +import 'package:over_react/over_react.dart'; +part 'invalid_gnerated_part_filename_parent.over_react.g.dart'; + +part 'invalid_generated_part_filename_part.dart'; diff --git a/tools/analyzer_plugin/playground/web/invalid_generated_part_filename_part.dart b/tools/analyzer_plugin/playground/web/invalid_generated_part_filename_part.dart new file mode 100644 index 000000000..02e7333db --- /dev/null +++ b/tools/analyzer_plugin/playground/web/invalid_generated_part_filename_part.dart @@ -0,0 +1,11 @@ +//part 'invaid_generated_part_filename_part.over_react.g.dart'; +part of invalid_generated_part_filename; + +UiFactory Foo = _$Foo; // ignore: undefined_identifier + +mixin FooProps on UiProps {} + +class FooComponent extends UiComponent2 { + @override + render() {} +} diff --git a/tools/analyzer_plugin/playground/web/invalid_generated_part_filename.dart b/tools/analyzer_plugin/playground/web/invalid_generated_part_filename_standalone.dart similarity index 76% rename from tools/analyzer_plugin/playground/web/invalid_generated_part_filename.dart rename to tools/analyzer_plugin/playground/web/invalid_generated_part_filename_standalone.dart index aedfc77d4..7acf828df 100644 --- a/tools/analyzer_plugin/playground/web/invalid_generated_part_filename.dart +++ b/tools/analyzer_plugin/playground/web/invalid_generated_part_filename_standalone.dart @@ -1,6 +1,6 @@ import 'package:over_react/over_react.dart'; -part 'invalid_generated_part_filename.over_react.g.dart'; +part 'invalid_generated_pat_filename_standalone.over_react.g.dart'; UiFactory Foo = _$Foo; // ignore: undefined_identifier