generator: normalize out dir before comparing it (#32)

Author: greenrobot-team

generator/lib/src/builder_dirs.dart

@@ -0,0 +1,30 @@
+import 'package:build/build.dart';
+import 'package:path/path.dart' as path;
+
+import 'config.dart';
+
+class BuilderDirs {
+  /// The normalized path to the source code root directory.
+  final String root;
+
+  /// The normalized path to the output directory for generated code.
+  final String out;
+
+  BuilderDirs._(this.root, this.out);
+
+  factory BuilderDirs(BuildStep buildStep, Config config) {
+    // Paths from Config are supplied by the user and may contain duplicate
+    // slashes or be empty: normalize all paths to not return duplicate or
+    // trailing slashes to ensure they can be compared via strings.
+    final root = path.normalize(path.dirname(buildStep.inputId.path));
+    final String out;
+    if (root.endsWith('test')) {
+      out = path.normalize('$root/${config.outDirTest}');
+    } else if (root.endsWith('lib')) {
+      out = path.normalize('$root/${config.outDirLib}');
+    } else {
+      throw ArgumentError('Is not lib or test directory: "$root"');
+    }
+    return BuilderDirs._(root, out);
+  }
+}

generator/lib/src/code_builder.dart

@@ -4,6 +4,7 @@ import 'dart:convert';
 
 import 'package:build/build.dart';
 import 'package:glob/glob.dart';
+import 'package:objectbox_generator/src/builder_dirs.dart';
 import 'package:path/path.dart' as path;
 import 'package:objectbox/internal.dart';
 import 'package:dart_style/dart_style.dart';
@@ -27,25 +28,14 @@ class CodeBuilder extends Builder {
     r'$test$': [path.join(_config.outDirTest, _config.codeFile)]
   };
 
-  String _dir(BuildStep buildStep) => path.dirname(buildStep.inputId.path);
-
-  String _outDir(BuildStep buildStep) {
-    var dir = _dir(buildStep);
-    if (dir.endsWith('test')) {
-      return dir + '/' + _config.outDirTest;
-    } else if (dir.endsWith('lib')) {
-      return dir + '/' + _config.outDirLib;
-    } else {
-      throw Exception('Unrecognized path being generated: $dir');
-    }
-  }
-
   @override
   FutureOr<void> build(BuildStep buildStep) async {
+    final builderDirs = BuilderDirs(buildStep, _config);
+
     // build() will be called only twice, once for the `lib` directory and once for the `test` directory
     // map from file name to a 'json' representation of entities
     final files = <String, List<dynamic>>{};
-    final glob = Glob(_dir(buildStep) + '/**' + EntityResolver.suffix);
+    final glob = Glob('${builderDirs.root}/**${EntityResolver.suffix}');
     await for (final input in buildStep.findAssets(glob)) {
       files[input.path] = json.decode(await buildStep.readAsString(input))!;
     }
@@ -64,26 +54,27 @@ class CodeBuilder extends Builder {
     log.info('Found ${entities.length} entities in: ${files.keys}');
 
     // update the model JSON with the read entities
-    final model = await updateModel(entities, buildStep);
+    final model = await updateModel(entities, buildStep, builderDirs);
 
     Pubspec? pubspec;
     try {
-      final pubspecFile = File(path.join(_dir(buildStep), '../pubspec.yaml'));
+      final pubspecFile = File(path.join(builderDirs.root, '../pubspec.yaml'));
       pubspec = Pubspec.parse(pubspecFile.readAsStringSync());
     } catch (e) {
       log.info("Couldn't load pubspec.yaml: $e");
     }
 
     // generate binding code
-    updateCode(model, files.keys.toList(growable: false), buildStep, pubspec);
+    updateCode(model, files.keys.toList(growable: false), buildStep,
+        builderDirs, pubspec);
   }
 
-  Future<ModelInfo> updateModel(
-      List<ModelEntity> entities, BuildStep buildStep) async {
+  Future<ModelInfo> updateModel(List<ModelEntity> entities, BuildStep buildStep,
+      BuilderDirs builderDirs) async {
     // load an existing model or initialize a new one
     ModelInfo model;
     final jsonId = AssetId(
-        buildStep.inputId.package, _outDir(buildStep) + '/' + _config.jsonFile);
+        buildStep.inputId.package, '${builderDirs.out}/${_config.jsonFile}');
     if (await buildStep.canRead(jsonId)) {
       log.info('Using model: ${jsonId.path}');
       model =
@@ -106,44 +97,54 @@ class CodeBuilder extends Builder {
     return model;
   }
 
-  void updateCode(ModelInfo model, List<String> infoFiles, BuildStep buildStep,
-      Pubspec? pubspec) async {
-    // If output directory is not package root directory,
-    // need to prefix imports with as many '../' to be relative from root.
-    final rootPath = _dir(buildStep);
-    final outPath = _outDir(buildStep);
-    final rootDir = Directory(rootPath).absolute;
-    var outDir = Directory(outPath).absolute;
-    var prefix = '';
+  /// Returns a prefix for imports if the output directory is not the
+  /// package root directory.
+  ///
+  /// Returns the empty string if the output directory is the root directory,
+  /// otherwise adds as many '../' as necessary to be relative from root.
+  ///
+  /// Returns null if the root directory was not found by walking up from the
+  /// output directory.
+  static String? getPrefixFor(BuilderDirs builderDirs) {
+    // Note: comparing path strings below so paths should be normalized (they
+    // are by BuilderDirs).
+    final rootDir = Directory(builderDirs.root).absolute;
+    var outDir = Directory(builderDirs.out).absolute;
 
     if (!outDir.path.startsWith(rootDir.path)) {
       throw InvalidGenerationSourceError(
           'configured output_dir ${outDir.path} is not a '
           'subdirectory of the source directory ${rootDir.path}');
     }
 
+    var prefix = '';
     while (outDir.path != rootDir.path) {
       final parent = outDir.parent;
       if (parent.path == outDir.path) {
-        log.warning(
-            'Failed to find package root from output directory, generated imports might be incorrect.');
-        prefix = '';
-        break; // Reached top-most directory, stop searching.
+        return null; // Reached top-most directory, stop searching.
       }
       outDir = parent;
       prefix += '../';
     }
-    if (prefix.isNotEmpty) {
+    return prefix;
+  }
+
+  void updateCode(ModelInfo model, List<String> infoFiles, BuildStep buildStep,
+      BuilderDirs builderDirs, Pubspec? pubspec) async {
+    var prefix = getPrefixFor(builderDirs);
+    if (prefix == null) {
+      log.warning(
+          'Failed to find package root from output directory, generated imports might be incorrect (rootDir="${builderDirs.root}", outDir="${builderDirs.out}")');
+    } else if (prefix.isNotEmpty) {
       log.info(
-          'Output directory not in package root, adding prefix to imports: ' +
-              prefix);
+          'Output directory not in package root, adding prefix to imports: $prefix');
     }
 
     // transform '/lib/path/entity.objectbox.info' to 'path/entity.dart'
     final imports = infoFiles
         .map((file) => file
             .replaceFirst(EntityResolver.suffix, '.dart')
-            .replaceFirst(rootPath + '/', prefix))
+            .replaceFirst('${builderDirs.root}/', prefix ?? ''))
         .toList();
 
     var code = CodeChunks.objectboxDart(model, imports, pubspec);
@@ -152,8 +153,8 @@ class CodeBuilder extends Builder {
       code = DartFormatter().format(code);
     } finally {
       // Write the code even after a formatter error so it's easier to debug.
-      final codeId =
-          AssetId(buildStep.inputId.package, outPath + '/' + _config.codeFile);
+      final codeId = AssetId(
+          buildStep.inputId.package, '${builderDirs.out}/${_config.codeFile}');
       log.info('Generating code: ${codeId.path}');
       await buildStep.writeAsString(codeId, code);
     }

generator/lib/src/config.dart

@@ -22,7 +22,7 @@ class Config {
   final String outDirLib;
   final String outDirTest;
 
-  Config._(
+  Config(
       {String? jsonFile,
       String? codeFile,
       String? outDirLib,
@@ -48,9 +48,9 @@ class Config {
           outDirLib = outDirTest = outDirYaml as String?;
         }
 
-        return Config._(outDirLib: outDirLib, outDirTest: outDirTest);
+        return Config(outDirLib: outDirLib, outDirTest: outDirTest);
       }
     }
-    return Config._();
+    return Config();
   }
 }

generator/pubspec.yaml

@@ -20,6 +20,7 @@ dependencies:
   yaml: ^3.0.0
 
 dev_dependencies:
+  test: ^1.16.5
   lints: ^2.0.1
 
 # NOTE: remove before publishing

generator/test/code_builder_test.dart

@@ -0,0 +1,163 @@
+import 'dart:async';
+import 'dart:convert';
+
+import 'package:objectbox_generator/src/builder_dirs.dart';
+import 'package:path/path.dart' as path;
+import 'package:build/build.dart';
+import 'package:build/src/builder/build_step_impl.dart';
+import 'package:crypto/src/digest.dart';
+import 'package:glob/glob.dart';
+import 'package:objectbox_generator/src/code_builder.dart';
+import 'package:objectbox_generator/src/config.dart';
+import 'package:source_gen/source_gen.dart';
+import 'package:test/test.dart';
+
+void main() {
+  var reader = StubAssetReader();
+  var writer = StubAssetWriter();
+  final resourceManager = ResourceManager();
+  // Default directory structure: sources inside lib folder.
+  final testBuildStep = BuildStepImpl(
+      AssetId("objectbox_generator_test", "lib/\$lib\$"),
+      [],
+      reader,
+      writer,
+      null,
+      resourceManager);
+
+  group('getRootDir and getOutDir', () {
+    test('lib', () {
+      final builderDirs = BuilderDirs(testBuildStep, Config());
+      expect(builderDirs.root, equals('lib'));
+      expect(builderDirs.out, equals('lib'));
+    });
+
+    test('test', () {
+      final testBuildStepTest = BuildStepImpl(
+          AssetId("objectbox_generator_test", "test/\$test\$"),
+          [],
+          reader,
+          writer,
+          null,
+          resourceManager);
+      final builderDirs = BuilderDirs(testBuildStepTest, Config());
+      expect(builderDirs.root, equals('test'));
+      expect(builderDirs.out, equals('test'));
+    });
+
+    test('not supported', () {
+      final testBuildStepNotSupported = BuildStepImpl(
+          AssetId("objectbox_generator_test", "custom/\$custom\$"),
+          [],
+          reader,
+          writer,
+          null,
+          resourceManager);
+      expect(
+          () => BuilderDirs(testBuildStepNotSupported, Config()),
+          throwsA(predicate((e) =>
+              e is ArgumentError &&
+              e.message == 'Is not lib or test directory: "custom"')));
+    });
+
+    test('out dir with redundant slash', () {
+      final builderDirs = BuilderDirs(testBuildStep, Config(outDirLib: '/'));
+      expect(builderDirs.root, equals('lib'));
+      expect(builderDirs.out, equals(path.normalize('lib')));
+    });
+
+    test('out dir not in root dir', () {
+      final builderDirs =
+          BuilderDirs(testBuildStep, Config(outDirLib: '../sibling'));
+      expect(builderDirs.root, equals('lib'));
+      expect(builderDirs.out, equals(path.normalize('sibling')));
+    });
+
+    test('out dir below root dir', () {
+      final builderDirs =
+          BuilderDirs(testBuildStep, Config(outDirLib: 'below/root'));
+      expect(builderDirs.root, equals('lib'));
+      expect(builderDirs.out, equals(path.normalize('lib/below/root')));
+    });
+  });
+
+  group('getPrefixFor', () {
+    test('out dir is root dir', () {
+      expect(CodeBuilder.getPrefixFor(BuilderDirs(testBuildStep, Config())),
+          equals(''));
+    });
+
+    test('out dir redundant slash', () {
+      expect(
+          CodeBuilder.getPrefixFor(
+              BuilderDirs(testBuildStep, Config(outDirLib: '/'))),
+          equals(''));
+      expect(
+          CodeBuilder.getPrefixFor(
+              BuilderDirs(testBuildStep, Config(outDirLib: '//below/'))),
+          equals('../'));
+    });
+
+    test('out dir not in root dir', () {
+      expect(
+          () => CodeBuilder.getPrefixFor(
+              BuilderDirs(testBuildStep, Config(outDirLib: '../sibling'))),
+          throwsA(predicate((e) =>
+              e is InvalidGenerationSourceError &&
+              e.message
+                  .contains("is not a subdirectory of the source directory"))));
+
+      expect(
+          () => CodeBuilder.getPrefixFor(
+              BuilderDirs(testBuildStep, Config(outDirLib: '../../above'))),
+          throwsA(predicate((e) =>
+              e is InvalidGenerationSourceError &&
+              e.message
+                  .contains("is not a subdirectory of the source directory"))));
+    });
+
+    test('out dir below root dir', () {
+      expect(
+          CodeBuilder.getPrefixFor(
+              BuilderDirs(testBuildStep, Config(outDirLib: 'below'))),
+          equals('../'));
+      expect(
+          CodeBuilder.getPrefixFor(
+              BuilderDirs(testBuildStep, Config(outDirLib: 'below/lower'))),
+          equals('../../'));
+    });
+  });
+}
+
+/// A no-op implementation of [AssetReader].
+class StubAssetReader extends AssetReader implements MultiPackageAssetReader {
+  StubAssetReader();
+
+  @override
+  Future<bool> canRead(AssetId id) => Future.value(false);
+
+  @override
+  Future<List<int>> readAsBytes(AssetId id) => Future.value([]);
+
+  @override
+  Future<String> readAsString(AssetId id, {Encoding encoding = utf8}) =>
+      Future.value('');
+
+  @override
+  Stream<AssetId> findAssets(Glob glob, {String? package}) =>
+      const Stream<Never>.empty();
+
+  @override
+  Future<Digest> digest(AssetId id) => Future.value(Digest([1, 2, 3]));
+}
+
+/// A no-op implementation of [AssetWriter].
+class StubAssetWriter implements AssetWriter {
+  const StubAssetWriter();
+
+  @override
+  Future writeAsBytes(_, __) => Future.value(null);
+
+  @override
+  Future writeAsString(_, __, {Encoding encoding = utf8}) => Future.value(null);
+}

objectbox/CHANGELOG.md

@@ -8,6 +8,8 @@
   final query = box.query().order(Person_.name).build();
   ```
 * Allow `analyzer` with major version 5. #487
+* Generator not longer warns that it can not find the package source root if the output directory is
+  the package root directory.
 
 ## 1.6.2 (2022-08-24)