[jnigen] Move output related configuration to single place (#93)

Author: mahesh-hegde

.github/workflows/test-package.yml

@@ -331,7 +331,7 @@ jobs:
       - run: flutter build apk
         working-directory: pkg/jnigen/example/notification_plugin/example
       - name: re-generate bindings
-        run: flutter pub run jnigen -Ddart_root=_dart -Dc_root=_c --config jnigen.yaml
+        run: flutter pub run jnigen -Doutput.dart.path=_dart -Doutput.c.path=_c --config jnigen.yaml
       - name: compare generated dart bindings
         run: diff -qr lib/ _dart
       - name: compare generated C bindings
@@ -361,7 +361,7 @@ jobs:
       - run: flutter analyze
       - run: flutter build apk
       - name: re-generate bindings
-        run: flutter pub run jnigen -Ddart_root=_dart -Dc_root=_c --config jnigen.yaml
+        run: flutter pub run jnigen -Doutput.dart.path=_dart -Doutput.c.path=_c --config jnigen.yaml
       - name: compare generated dart bindings
         run: diff -qr lib/android_utils _dart
       - name: compare generated C bindings
@@ -390,7 +390,7 @@ jobs:
       - run: dart pub get
       - name: Generate bindings
         run: |
-          dart run jnigen -Dc_root=_c -Ddart_root=_dart --config jnigen.yaml
+          dart run jnigen -Doutput.c.path=_c -Doutput.dart.path=_dart --config jnigen.yaml
       - name: Compare generated bindings
         run: |
           diff -qr _c src/

pkgs/jnigen/example/in_app_java/jnigen.yaml

@@ -1,11 +1,14 @@
 android_sdk_config:
   add_gradle_deps: true
 
-library_name: android_utils
+output:
+  c:
+    library_name: android_utils
+    path: src/android_utils
+  dart:
+    path: lib/android_utils
+
 source_path:
   - 'android/app/src/main/java'
 classes:
   - 'com.example.in_app_java.AndroidUtils'
-c_root: src/android_utils
-dart_root: lib/android_utils
-root_package: com.example

pkgs/jnigen/example/in_app_java/tool/generate_bindings.dart

@@ -7,10 +7,14 @@ preamble: |
   // for details. All rights reserved. Use of this source code is governed by a
   // BSD-style license that can be found in the LICENSE file.
 
-library_name: notification_plugin
 source_path:
   - 'android/src/main/java'
 classes:
   - 'com.example.notification_plugin.Notifications'
-c_root: src/
-dart_root: lib/
+
+output:
+  c:
+    path: 'src/'
+    library_name: notification_plugin
+  dart:
+    path: 'lib/'

pkgs/jnigen/example/notification_plugin/jnigen.yaml

@@ -1,7 +1,3 @@
-## Name of the generated library, this is required and used as the name of
-## shared library which contains C bindings
-library_name: pdfbox_plugin
-
 ## String to be pasted verbatim into generated bindings
 preamble: |
   // Generated from Apache PDFBox library which is licensed under the Apache License 2.0.
@@ -22,18 +18,24 @@ preamble: |
   // See the License for the specific language governing permissions and
   // limitations under the License.
 
-## Root for generated C bindings.
-c_root: 'src/'
-
-## C files can be stored in a different sub-directory inside c_root.
-##
-## We have a guideline to keep all generated code in third_party/ since original
-## project's license applies to generated code. So we specify third_party/ as
-## c_subdir while keeping generated CMakeLists.txt in src/.
-c_subdir: 'third_party/'
-
-## Root for generated Dart bindings.
-dart_root: 'lib/src/third_party/'
+## Output configuration
+output:
+  c:
+    ## Path to write generated C bindings
+    path: 'src/'
+    ## C files can be stored in a different sub-directory inside root.
+    ##
+    ## We have a guideline to keep all generated code in third_party/ since original
+    ## project's license applies to generated code. So we specify third_party/ as
+    ## c_subdir while keeping generated CMakeLists.txt in src/.
+    subdir: 'third_party/'
+    ## Name of the generated library. This is a required parameter, and used for the name of the
+    ## shared library and CMake configuration.
+    library_name: 'pdfbox_plugin'
+  dart:
+    ## Generated dart bindings will be written to this path. They will follow the same folder hierarchy
+    ## as the original Java code.
+    path: 'lib/src/third_party/'
 
 ## Classes / packages for which bindings need to be generated.
 classes:

pkgs/jnigen/example/pdfbox_plugin/jnigen.yaml

@@ -14,13 +14,7 @@ abstract class ApiPreprocessor {
   static void preprocessAll(Map<String, ClassDecl> classes, Config config,
       {bool renameClasses = false}) {
     final Map<String, int> classNameCounts = {};
-    final rootPackage = config.rootPackage;
     for (var c in classes.values) {
-      final packageName = c.packageName;
-      if (rootPackage != null && !packageName.startsWith('$rootPackage.')) {
-        throw ArgumentError("class ${c.binaryName} not in "
-            "root package $rootPackage");
-      }
       final className = getSimplifiedClassName(c.binaryName);
       c.uniqueName = renameConflict(classNameCounts, className);
       if (renameClasses) {

pkgs/jnigen/lib/src/bindings/preprocessor.dart

@@ -125,39 +125,59 @@ enum SummarizerBackend {
   doclet,
 }
 
+class CCodeOutputConfig {
+  CCodeOutputConfig({
+    required this.path,
+    required this.libraryName,
+    this.subdir,
+  });
+
+  /// Directory to write JNI C Bindings, in C+Dart mode.
+  ///
+  /// Strictly speaking, this is the root to place the `CMakeLists.txt` file
+  /// for the generated C bindings. It may be desirable to use the [subdir]
+  /// options to write C files to a subdirectory of [path]. For instance,
+  /// when generated code is required to be in `third_party` directory.
+  Uri path;
+
+  /// Name of generated library in CMakeLists.txt configuration.
+  ///
+  /// This will also determine the name of shared object file.
+  String libraryName;
+
+  /// Subfolder relative to [path] to write generated C code.
+  String? subdir;
+}
+
+class DartCodeOutputConfig {
+  // TODO(#90): Support output_structure = single_file | package_structure.
+
+  DartCodeOutputConfig({required this.path});
+
+  /// Path to write generated Dart bindings.
+  Uri path;
+}
+
+class OutputConfig {
+  // TODO(#60): Add bindings_type = dart_only | c_based.
+
+  OutputConfig({required this.cConfig, required this.dartConfig});
+  DartCodeOutputConfig dartConfig;
+  CCodeOutputConfig cConfig;
+}
+
 class BindingExclusions {
   BindingExclusions({this.methods, this.fields, this.classes});
   MethodFilter? methods;
   FieldFilter? fields;
   ClassFilter? classes;
 }
 
-enum BindingsType {
-  cBased, // C+Dart bindings
-  singleFile,
-  packageStructured,
-}
-
-BindingsType getBindingsType(String? type, BindingsType defaultType) {
-  const names = {
-    'c_based': BindingsType.cBased,
-    'single_file': BindingsType.singleFile,
-    'package_structured': BindingsType.packageStructured,
-  };
-  return names[type] ?? defaultType;
-}
-
 /// Configuration for jnigen binding generation.
 class Config {
   Config({
+    required this.outputConfig,
     required this.classes,
-    this.bindingsType = BindingsType.cBased,
-    this.outputPath,
-    this.libraryName,
-    this.cRoot,
-    this.dartRoot,
-    this.cSubdir,
-    this.rootPackage,
     this.exclude,
     this.sourcePath,
     this.classPath,
@@ -168,16 +188,10 @@ class Config {
     this.summarizerOptions,
     this.logLevel = Level.INFO,
     this.dumpJsonTo,
-  }) {
-    if (bindingsType == BindingsType.cBased) {
-      if (cRoot == null || dartRoot == null || libraryName == null) {
-        throw ArgumentError("In c_based mode these values must be specified: "
-            "c_root, dart_root, library_name");
-      }
-    } else {
-      throw UnimplementedError("BindingsType not yet supported: $bindingsType");
-    }
-  }
+  });
+
+  /// Output configuration for generated bindings
+  OutputConfig outputConfig;
 
   /// List of classes or packages for which bindings have to be generated.
   ///
@@ -188,41 +202,6 @@ class Config {
   /// name suffix is `.class`.
   List<String> classes;
 
-  /// Type of bindings to generate.
-  final BindingsType bindingsType;
-
-  /// Name of generated library in CMakeLists.txt configuration.
-  ///
-  /// This will also determine the name of shared object file.
-  final String? libraryName;
-
-  /// Directory to write JNI C Bindings, in C+Dart mode.
-  ///
-  /// Strictly speaking, this is the root to place the `CMakeLists.txt` file
-  /// for the generated C bindings. It may be desirable to use the [cSubdir]
-  /// options to write C files to a subdirectory of [cRoot]. For instance,
-  /// when generated code is required to be in `third_party` directory.
-  Uri? cRoot;
-
-  /// Directory to write Dart bindings, in C + Dart mode.
-  Uri? dartRoot;
-
-  /// Subfolder relative to [cRoot] to write generated C code.
-  String? cSubdir;
-
-  /// Java package corresponding to the dart_root directory.
-  ///
-  /// By default, the complete java hierarchy is mirrored. For instance,
-  /// `org.apache.pdfbox.text` becomes `org/apache/pdfbox/text.dart`.
-  /// This is often undesirable, when all packages have a common package. In
-  /// such cases, a super-package name can be provided. This will be assumed as
-  /// the prefix of all packages and hierarchy will be created relative to this
-  /// package.
-  String? rootPackage;
-
-  /// Output file or folder in non-legacy modes
-  Uri? outputPath;
-
   /// Methods and fields to be excluded from generated bindings.
   final BindingExclusions? exclude;
 
@@ -288,7 +267,6 @@ class Config {
       return res;
     }
 
-    Uri? fileUri(String? path) => path != null ? Uri.file(path) : null;
     Uri? directoryUri(String? path) =>
         path != null ? Uri.directory(path) : null;
 
@@ -339,15 +317,17 @@ class Config {
         methods: regexFilter<Method>(_Props.excludeMethods),
         fields: regexFilter<Field>(_Props.excludeFields),
       ),
-      bindingsType: getBindingsType(
-          prov.getString(_Props.bindingsType), BindingsType.cBased),
-      cRoot: directoryUri(prov.getString(_Props.cRoot)),
-      dartRoot: directoryUri(prov.getString(_Props.dartRoot)),
-      outputPath: fileUri(prov.getString(_Props.outputPath)),
-      cSubdir: prov.getString(_Props.cSubdir),
-      rootPackage: prov.getString(_Props.rootPackage),
+      outputConfig: OutputConfig(
+        cConfig: CCodeOutputConfig(
+          libraryName: must(prov.getString, '', _Props.libraryName),
+          path: Uri.directory(must(prov.getString, '.', _Props.cRoot)),
+          subdir: prov.getString(_Props.cSubdir),
+        ),
+        dartConfig: DartCodeOutputConfig(
+          path: Uri.directory(must(prov.getString, '.', _Props.dartRoot)),
+        ),
+      ),
       preamble: prov.getString(_Props.preamble),
-      libraryName: must(prov.getString, '', _Props.libraryName),
       importMap: prov.getStringMap(_Props.importMap),
       mavenDownloads: prov.hasValue(_Props.mavenDownloads)
           ? MavenDownloads(
@@ -411,14 +391,14 @@ class _Props {
   static const excludeFields = '$exclude.fields';
 
   static const importMap = 'import_map';
-  static const outputPath = 'output_path';
-  static const bindingsType = 'bindings_type';
-  static const dartRoot = 'dart_root';
-  static const cRoot = 'c_root';
-  static const cSubdir = 'c_subdir';
-  static const rootPackage = 'root_package';
+  static const outputConfig = 'output';
+  static const cCodeOutputConfig = '$outputConfig.c';
+  static const dartCodeOutputConfig = '$outputConfig.dart';
+  static const cRoot = '$cCodeOutputConfig.path';
+  static const cSubdir = '$cCodeOutputConfig.subdir';
+  static const dartRoot = '$dartCodeOutputConfig.path';
+  static const libraryName = '$cCodeOutputConfig.library_name';
   static const preamble = 'preamble';
-  static const libraryName = 'library_name';
   static const logLevel = 'log_level';
 
   static const mavenDownloads = 'maven_downloads';

pkgs/jnigen/lib/src/config/config.dart

@@ -178,11 +178,11 @@ class FilesWriter extends BindingsWriter {
     }
     final classNames = classesByName.keys.toSet();
 
-    final cRoot = config.cRoot!;
+    final cRoot = config.outputConfig.cConfig.path;
     log.info("Using c root = $cRoot");
-    final dartRoot = config.dartRoot!;
+    final dartRoot = config.outputConfig.dartConfig.path;
     log.info("Using dart root = $dartRoot");
-    final libraryName = config.libraryName!;
+    final libraryName = config.outputConfig.cConfig.libraryName;
 
     log.info('Creating dart init file ...');
     final initFileUri = dartRoot.resolve(_initFileName);
@@ -192,7 +192,7 @@ class FilesWriter extends BindingsWriter {
       initCode = '$preamble\n$initCode';
     }
     await initFile.writeAsString(initCode, flush: true);
-    final subdir = config.cSubdir ?? '.';
+    final subdir = config.outputConfig.cConfig.subdir ?? '.';
     final cFileRelativePath = '$subdir/$libraryName.c';
     final cFile = await File.fromUri(cRoot.resolve(cFileRelativePath))
         .create(recursive: true);

pkgs/jnigen/lib/src/writers/files_writer.dart

@@ -21,9 +21,13 @@ final testSrc = join(thirdParty, 'test_', 'src');
 /// two fields are not equal.
 void expectConfigsAreEqual(Config a, Config b) {
   expect(a.classes, equals(b.classes), reason: "classes");
-  expect(a.libraryName, equals(b.libraryName), reason: "libraryName");
-  expect(a.cRoot, equals(b.cRoot), reason: "cRoot");
-  expect(a.dartRoot, equals(b.dartRoot), reason: "dartRoot");
+  expect(a.outputConfig.cConfig.libraryName,
+      equals(b.outputConfig.cConfig.libraryName),
+      reason: "libraryName");
+  expect(a.outputConfig.cConfig.path, equals(b.outputConfig.cConfig.path),
+      reason: "cRoot");
+  expect(a.outputConfig.dartConfig.path, equals(b.outputConfig.dartConfig.path),
+      reason: "dartRoot");
   expect(a.sourcePath, equals(b.sourcePath), reason: "sourcePath");
   expect(a.classPath, equals(b.classPath), reason: "classPath");
   expect(a.preamble, equals(b.preamble), reason: "preamble");
@@ -71,8 +75,8 @@ void main() {
   final config = Config.parseArgs([
     '--config',
     join(jacksonCoreTests, 'jnigen.yaml'),
-    '-Dc_root=$testSrc',
-    '-Ddart_root=$testLib',
+    '-Doutput.c.path=$testSrc',
+    '-Doutput.dart.path=$testLib',
   ]);
 
   test('compare configuration values', () {