Merge branch 'gh-486-fix-pkg-root-detector' into 'main'

Generator: do not warn if out dir is package root.

See merge request objectbox/objectbox-dart!25

Author: greenrobot-team

.github/workflows/dart.yml

@@ -21,7 +21,7 @@ jobs:
       - uses: actions/checkout@v3
       - name: Install ObjectBox C-API
         run: ./install.sh
-      - name: Test
+      - name: Integration test
         run: ./generator/test.sh
 
   # make sure the init script doesn't stop working - it's not something we usually run during normal development
@@ -54,10 +54,15 @@ jobs:
         with:
           sdk: ${{ matrix.sdk }}
       - uses: actions/checkout@v3
+      - name: Generator Test
+        working-directory: generator
+        run: |
+          ../tool/pub.sh get
+          ../tool/pub.sh run test
       - name: Install ObjectBox C-API
         working-directory: objectbox
         run: ../install.sh
-      - name: Test
+      - name: Library Test
         working-directory: objectbox
         run: |
           ../tool/pub.sh get

.gitlab-ci.yml

@@ -35,7 +35,7 @@ analyze:
     - cd ..
     - dart format --set-exit-if-changed --fix .
 
-# Runs generator integration tests, e.g. ensures generator works as expected.
+# Runs generator unit and integration tests, e.g. ensures generator works as expected.
 test-generator:linux:x64:
   extends: .cache
   stage: test
@@ -44,6 +44,9 @@ test-generator:linux:x64:
   script:
     - ./install.sh
     - ./generator/test.sh
+    - cd generator
+    - dart pub get
+    - dart test
 
 .test:
   extends: .cache

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