Address review comments.

Author: davidmorgan

build_config/README.md

@@ -273,6 +273,7 @@ triggers:
 
 An `annotation` trigger causes the builder to run if an annotation is used.
 So, `- annotation MyAnnotation` is a check for `@MyAnnotation` being used.
+A part file included from a library is also checked for the annotation.
 
 An `import` trigger says that the builder runs if there is a direct import
 of the specified library. This might be useful if a builder can run on code

build_runner_core/lib/src/generate/build.dart

@@ -5,6 +5,8 @@
 import 'dart:async';
 import 'dart:convert';
 
+import 'package:analyzer/dart/analysis/utilities.dart';
+import 'package:analyzer/dart/ast/ast.dart';
 import 'package:build/build.dart';
 // ignore: implementation_imports
 import 'package:build/src/internal.dart';
@@ -589,14 +591,47 @@ class Build {
       return false;
     }
     final primaryInputSource = await readerWriter.readAsString(primaryInput);
+    final compilationUnit = _parseCompilationUnit(primaryInputSource);
+    List<CompilationUnit>? compilationUnits;
     for (final trigger in buildTriggers) {
-      if (trigger.triggersOnPrimaryInput(primaryInputSource)) {
-        return true;
+      if (trigger.checksParts) {
+        compilationUnits ??= await _readAndParseCompilationUnits(
+          readerWriter,
+          primaryInput,
+          compilationUnit,
+        );
+        if (trigger.triggersOn(compilationUnits)) return true;
+      } else {
+        if (trigger.triggersOn([compilationUnit])) return true;
       }
     }
     return false;
   }
 
+  /// TODO(davidmorgan): cache parse results, share with deps parsing and
+  /// builder parsing.
+  static CompilationUnit _parseCompilationUnit(String content) {
+    return parseString(content: content, throwIfDiagnostics: false).unit;
+  }
+
+  static Future<List<CompilationUnit>> _readAndParseCompilationUnits(
+    AssetReader reader,
+    AssetId id,
+    CompilationUnit compilationUnit,
+  ) async {
+    final result = [compilationUnit];
+    for (var directive in compilationUnit.directives) {
+      if (directive is! PartDirective) continue;
+      final partId = AssetId.resolve(
+        Uri.parse(directive.uri.stringValue!),
+        from: id,
+      );
+      if (!await reader.canRead(partId)) continue;
+      result.add(_parseCompilationUnit(await reader.readAsString(partId)));
+    }
+    return result;
+  }
+
   Future<Iterable<AssetId>> _runPostBuildPhase(
     int phaseNum,
     PostBuildPhase phase,

build_runner_core/lib/src/package_graph/build_triggers.dart

@@ -4,6 +4,7 @@
 
 import 'dart:convert';
 
+import 'package:analyzer/dart/ast/ast.dart';
 import 'package:build_config/build_config.dart';
 import 'package:built_collection/built_collection.dart';
 import 'package:built_value/built_value.dart';
@@ -80,7 +81,7 @@ class BuildTriggers {
   /// Parses [triggers], or throws a descriptive exception on failure.
   ///
   /// [triggers] is an `Object` read directly from yaml, to be valid it should
-  /// be a list of strings that parse with [BuildTrigger.tryParse].
+  /// be a list of strings that parse with [BuildTrigger._tryParse].
   static (Iterable<BuildTrigger>, List<String>) parseList({
     required Object triggers,
   }) {
@@ -96,7 +97,7 @@ class BuildTriggers {
       BuildTrigger? trigger;
       String? warning;
       if (triggerString is String) {
-        (trigger, warning) = BuildTrigger.tryParse(triggerString);
+        (trigger, warning) = BuildTrigger._tryParse(triggerString);
       }
       if (trigger != null) {
         result.add(trigger);
@@ -124,13 +125,13 @@ class BuildTriggers {
 }
 
 /// A build trigger: a heuristic that possibly skips running a build step based
-/// on the primary input source.
+/// on the parsed primary input.
 abstract class BuildTrigger {
   /// Parses a trigger, returning `null` on failure, optionally with a warning
   /// message.
   ///
   /// The only supported trigger is [ImportBuildTrigger].
-  static (BuildTrigger?, String?) tryParse(String trigger) {
+  static (BuildTrigger?, String?) _tryParse(String trigger) {
     if (trigger.startsWith('import ')) {
       trigger = trigger.substring('import '.length);
       final result = ImportBuildTrigger(trigger);
@@ -143,18 +144,21 @@ abstract class BuildTrigger {
     return (null, 'Invalid trigger: `$trigger`');
   }
 
-  bool triggersOnPrimaryInput(String source);
+  /// Whether the trigger matches on any of [compilationUnits].
+  ///
+  /// This will be called either with the primary input compilation unit or all
+  /// compilation units (parts) depending on [checksParts].
+  bool triggersOn(List<CompilationUnit> compilationUnits);
+
+  /// Whether [triggersOn] should be called with all compilation units, not just
+  /// the primary input.
+  bool get checksParts;
 }
 
 // Note: `built_value` generated toString is relied on for digests, and
 // equality for testing.
 
 /// A build trigger that checks for a direct import.
-///
-/// TODO(davidmorgan): this currently over-matches, it just checks for a
-/// matching substring. It's a heuristic so over matching is okay, but it might
-/// be better to actually parse directives and do a correct match because
-/// directives are needed when resolving source.
 abstract class ImportBuildTrigger
     implements
         Built<ImportBuildTrigger, ImportBuildTriggerBuilder>,
@@ -166,18 +170,28 @@ abstract class ImportBuildTrigger
   factory ImportBuildTrigger(String import) =>
       _$ImportBuildTrigger._(import: import);
 
+  @memoized
+  String get packageImport => 'package:$import';
+
   @override
-  bool triggersOnPrimaryInput(String source) {
-    return source.contains('package:$import');
+  bool get checksParts => false;
+
+  @override
+  bool triggersOn(List<CompilationUnit> compilationUnits) {
+    for (final compilationUnit in compilationUnits) {
+      for (final directive in compilationUnit.directives) {
+        if (directive is! ImportDirective) continue;
+        if (directive.uri.stringValue == packageImport) return true;
+      }
+    }
+    return false;
   }
 
   String? get warning =>
       import.contains(_regexp) ? null : 'Invalid import trigger: `$import`';
 }
 
 /// A build trigger that checks for an annotation.
-///
-/// TODO(davidmorgan): like `ImportBuildTrigger` this over-matches.
 abstract class AnnotationBuildTrigger
     implements
         Built<AnnotationBuildTrigger, AnnotationBuildTriggerBuilder>,
@@ -190,8 +204,27 @@ abstract class AnnotationBuildTrigger
       _$AnnotationBuildTrigger._(annotation: annotation);
 
   @override
-  bool triggersOnPrimaryInput(String source) {
-    return source.contains('@$annotation');
+  bool get checksParts => true;
+
+  @override
+  bool triggersOn(List<CompilationUnit> compilationUnits) {
+    for (final compilationUnit in compilationUnits) {
+      for (final declaration in compilationUnit.declarations) {
+        for (final metadata in declaration.metadata) {
+          var name = metadata.name.name;
+          if (metadata.constructorName != null) {
+            name = '$name.${metadata.constructorName}';
+          }
+          if (annotation == name) return true;
+          final periodIndex = name.indexOf('.');
+          if (periodIndex != -1) {
+            name = name.substring(periodIndex + 1);
+            if (annotation == name) return true;
+          }
+        }
+      }
+    }
+    return false;
   }
 
   String? get warning =>

build_runner_core/lib/src/package_graph/build_triggers.g.dart

@@ -67,7 +67,7 @@ triggers:
     - import trigger/trigger.dart
 ''');
         tester.importGraph({
-          'a.1': ['package:trigger/trigger.dart'],
+          'a.1': ['package:trigger/trigger'],
         });
       });
 
@@ -122,7 +122,7 @@ triggers:
 ''');
         await tester.build();
         tester.importGraph({
-          'a.1': ['package:trigger/trigger.dart'],
+          'a.1': ['package:trigger/trigger'],
         });
         expect(await tester.build(change: 'a.1'), Result(written: ['a.2']));
       });
@@ -138,7 +138,7 @@ targets:
           run_only_if_triggered: true
 ''');
         tester.importGraph({
-          'a.1': ['package:trigger/trigger.dart'],
+          'a.1': ['package:trigger/trigger'],
         });
         await tester.build();
         expect(
@@ -230,7 +230,7 @@ triggers:
     - import trigger/trigger.dart
 ''');
         tester.importGraph({
-          'a.1': ['package:trigger/trigger.dart'],
+          'a.1': ['package:trigger/trigger'],
         });
         await tester.build();
         tester.importGraph({'a.1': []});
@@ -252,7 +252,7 @@ triggers:
     - import trigger/trigger.dart
 ''');
         tester.importGraph({
-          'a.1': ['package:trigger/trigger.dart'],
+          'a.1': ['package:trigger/trigger'],
         });
         await tester.build();
         expect(