Skip to content

[pigeon] Relocates generator classes and updates imports #2985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 5 commits into from
Dec 20, 2022
Merged

[pigeon] Relocates generator classes and updates imports #2985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
508c182
start
tarrinneal Dec 20, 2022
9c3023f
Class file transfer and skeleton comments
tarrinneal Dec 20, 2022
99c5c01
relocate generator classes and update imports
tarrinneal Dec 20, 2022
68c3511
Merge branch 'main' of github.com:flutter/packages into skeleton
tarrinneal Dec 20, 2022
67fd994
merge main and changelog update
tarrinneal Dec 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/pigeon/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 4.2.15

* Relocates generator classes.

## 4.2.14

* [c++] Fixes reply sending non EncodableValue wrapped lists.
Expand Down
54 changes: 53 additions & 1 deletion packages/pigeon/lib/cpp_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io';
import 'package:path/path.dart' as path;

import 'ast.dart';
import 'functional.dart';
import 'generator.dart';
import 'generator_tools.dart';
import 'pigeon_lib.dart' show Error;
import 'pigeon_lib.dart' show Error, PigeonOptions, lineReader, openSink;
Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should ideally move these two utility methods to a utility file, and find somewhere for the other things to live, so we don't have circular includes. Than can be done later though.

Copy link
Member

@gaaclarke gaaclarke Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason that the Generator was in the place it was previously is that it was keeping PigeonOptions out of the generator files. The idea being that the generators are cleaner/safer/easier-to-maintain if they only have access to the options designated for them instead of giving them every single option. The idea of the Generator was more of an adapter of the public functions in the generator files which were the actual generators.

If you wanted to retain that organization it's going to be more difficult if you move the Generator implementations to the generator files. You'll have to make Generator generic:

abstract class Generator<T> {
  void generate(StringSink sink, T options, Root root);
  List<Error> validate(T options, Root root);
}

Then when you store adapters between PigeonOptions and the input that the Generator needs where you store all your generators.

/// Adapts a Generator to PigeonOptions
class GeneratorAdapter {
  GeneratorAdapter({this.generate, this.shouldGenerate, this.validate});
  void Function(StringSink sink, PigeonOptions options, Root root) generate;
  IOSink? Function(PigeonOptions options) shouldGenerate;
  List<Error> Function(PigeonOptions options, Root root) validate;
}

var generator = Generator();
generators.add(GeneratorAdapter(
  GeneratorAdapter(
    generate:(sink, options, root) => generator.generate(sink, options.fooOptions, root),
    shouldGenerate:(options) => _openSink(options.fooOut),
    validate:(options, root) => generator.validate(options.fooOptions, root)));

The benefits being:

  1. it makes the Generator API more simple
  2. it separates the idea of generating code and adapting it to the PigeonOptions format
  3. it keeps PigeonOptions out of the generator files

How do you like that?

Copy link
Member

@gaaclarke gaaclarke Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sonofabitch, late comment. For architecture questions can you guys touch base first. I didn't really have an opportunity to react to this.

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can revert+retract if this is an issue. I didn't expect putting the generator classes in the generator files to be controversial.

The idea of the Generator was more of an adapter of the public functions in the generator files which were the actual generators.

This is why naming is important; I didn't expect that in a package where the most important conceptual thing are generators that the Generator class wasn't intended to represent a generator.

Copy link
Member

@gaaclarke gaaclarke Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, it should have been "_Generator" but was made public to help with testing. We don't need to retract. I think separating the adapter and the generator makes everything tidier and easier to maintain but we can do that in a different PR. I think having a Generator abstract class that people follow along in the generator files makes it a bit easier conceptually instead of having loose functions, so that is an improvement.


/// General comment opening token.
const String _commentPrefix = '//';
Expand Down Expand Up @@ -64,6 +68,54 @@ class CppOptions {
}
}

/// A [Generator] that generates C++ header code.
class CppHeaderGenerator implements Generator {
/// Constructor for [CppHeaderGenerator].
const CppHeaderGenerator();

@override
void generate(StringSink sink, PigeonOptions options, Root root) {
final CppOptions cppOptions = options.cppOptions ?? const CppOptions();
final CppOptions cppOptionsWithHeader = cppOptions.merge(CppOptions(
copyrightHeader: options.copyrightHeader != null
? lineReader(options.copyrightHeader!)
: null));
generateCppHeader(path.basenameWithoutExtension(options.cppHeaderOut!),
cppOptionsWithHeader, root, sink);
}

@override
IOSink? shouldGenerate(PigeonOptions options) =>
openSink(options.cppHeaderOut);

@override
List<Error> validate(PigeonOptions options, Root root) =>
validateCpp(options.cppOptions!, root);
}

/// A [Generator] that generates C++ source code.
class CppSourceGenerator implements Generator {
/// Constructor for [CppSourceGenerator].
const CppSourceGenerator();

@override
void generate(StringSink sink, PigeonOptions options, Root root) {
final CppOptions cppOptions = options.cppOptions ?? const CppOptions();
final CppOptions cppOptionsWithHeader = cppOptions.merge(CppOptions(
copyrightHeader: options.copyrightHeader != null
? lineReader(options.copyrightHeader!)
: null));
generateCppSource(cppOptionsWithHeader, root, sink);
}

@override
IOSink? shouldGenerate(PigeonOptions options) =>
openSink(options.cppSourceOut);

@override
List<Error> validate(PigeonOptions options, Root root) => <Error>[];
}

String _getCodecSerializerName(Api api) => '${api.name}CodecSerializer';

const String _pointerPrefix = 'pointer';
Expand Down
62 changes: 61 additions & 1 deletion packages/pigeon/lib/dart_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io' show Directory, File, FileSystemEntity;
import 'dart:io' show Directory, File, FileSystemEntity, IOSink;

import 'package:path/path.dart' as path;
import 'package:yaml/yaml.dart' as yaml;

import 'ast.dart';
import 'functional.dart';
import 'generator.dart';
import 'generator_tools.dart';
import 'pigeon_lib.dart' show Error, PigeonOptions, lineReader, openSink;

/// Documentation comment open symbol.
const String _docCommentPrefix = '///';
Expand Down Expand Up @@ -65,6 +67,64 @@ String _escapeForDartSingleQuotedString(String raw) {
/// Calculates the name of the codec class that will be generated for [api].
String _getCodecName(Api api) => '_${api.name}Codec';

/// A [Generator] that generates Dart source code.
class DartGenerator implements Generator {
/// Constructor for [DartGenerator].
const DartGenerator();

@override
void generate(StringSink sink, PigeonOptions options, Root root) {
final DartOptions dartOptionsWithHeader = _dartOptionsWithCopyrightHeader(
options.dartOptions, options.copyrightHeader);
generateDart(dartOptionsWithHeader, root, sink);
}

@override
IOSink? shouldGenerate(PigeonOptions options) => openSink(options.dartOut);

@override
List<Error> validate(PigeonOptions options, Root root) => <Error>[];
}

DartOptions _dartOptionsWithCopyrightHeader(
DartOptions? dartOptions, String? copyrightHeader) {
dartOptions = dartOptions ?? const DartOptions();
return dartOptions.merge(DartOptions(
copyrightHeader:
copyrightHeader != null ? lineReader(copyrightHeader) : null));
}

/// A [Generator] that generates Dart test source code.
class DartTestGenerator implements Generator {
/// Constructor for [DartTestGenerator].
const DartTestGenerator();

@override
void generate(StringSink sink, PigeonOptions options, Root root) {
final DartOptions dartOptionsWithHeader = _dartOptionsWithCopyrightHeader(
options.dartOptions, options.copyrightHeader);
generateTestDart(
dartOptionsWithHeader,
root,
sink,
dartOutPath: options.dartOut!,
testOutPath: options.dartTestOut!,
);
}

@override
IOSink? shouldGenerate(PigeonOptions options) {
if (options.dartTestOut != null) {
return openSink(options.dartTestOut);
} else {
return null;
}
}

@override
List<Error> validate(PigeonOptions options, Root root) => <Error>[];
}

/// Writes the codec that will be used by [api].
/// Example:
///
Expand Down
26 changes: 26 additions & 0 deletions packages/pigeon/lib/generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io';

import 'ast.dart';
import 'pigeon_lib.dart' show Error, PigeonOptions;

/// A generator that will write code to a sink based on the contents of [PigeonOptions].
abstract class Generator {
/// Constructor for Generator.
Generator();

/// Returns an [IOSink] instance to be written to if the [Generator] should
/// generate. If it returns `null`, the [Generator] will be skipped.
IOSink? shouldGenerate(PigeonOptions options);

/// Write the generated code described in [root] to [sink] using the
/// [options].
void generate(StringSink sink, PigeonOptions options, Root root);

/// Generates errors that would only be appropriate for this [Generator]. For
/// example, maybe a certain feature isn't implemented in a [Generator] yet.
List<Error> validate(PigeonOptions options, Root root);
}
2 changes: 1 addition & 1 deletion packages/pigeon/lib/generator_tools.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import 'dart:mirrors';
import 'ast.dart';

/// The current version of pigeon. This must match the version in pubspec.yaml.
const String pigeonVersion = '4.2.14';
const String pigeonVersion = '4.2.15';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
32 changes: 31 additions & 1 deletion packages/pigeon/lib/java_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io';

import 'package:path/path.dart' as path;

import 'ast.dart';
import 'functional.dart';
import 'generator.dart';
import 'generator_tools.dart';
import 'pigeon_lib.dart' show TaskQueueType;
import 'pigeon_lib.dart'
show Error, PigeonOptions, TaskQueueType, lineReader, openSink;

/// Documentation open symbol.
const String _docCommentPrefix = '/**';
Expand Down Expand Up @@ -84,6 +90,30 @@ class JavaOptions {
}
}

/// A [Generator] that generates Java source code.
class JavaGenerator implements Generator {
/// Constructor for [JavaGenerator].
const JavaGenerator();

@override
void generate(StringSink sink, PigeonOptions options, Root root) {
JavaOptions javaOptions = options.javaOptions ?? const JavaOptions();
javaOptions = javaOptions.merge(JavaOptions(
className: javaOptions.className ??
path.basenameWithoutExtension(options.javaOut!),
copyrightHeader: options.copyrightHeader != null
? lineReader(options.copyrightHeader!)
: null));
generateJava(javaOptions, root, sink);
}

@override
IOSink? shouldGenerate(PigeonOptions options) => openSink(options.javaOut);

@override
List<Error> validate(PigeonOptions options, Root root) => <Error>[];
}

/// Calculates the name of the codec that will be generated for [api].
String _getCodecName(Api api) => '${api.name}Codec';

Expand Down
29 changes: 28 additions & 1 deletion packages/pigeon/lib/kotlin_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io';

import 'ast.dart';
import 'functional.dart';
import 'generator.dart';
import 'generator_tools.dart';
import 'pigeon_lib.dart' show TaskQueueType;
import 'pigeon_lib.dart'
show Error, PigeonOptions, TaskQueueType, lineReader, openSink;

/// Documentation open symbol.
const String _docCommentPrefix = '/**';
Expand Down Expand Up @@ -64,6 +68,29 @@ class KotlinOptions {
}
}

/// A [Generator] that generates Kotlin source code.
class KotlinGenerator implements Generator {
/// Constructor for [KotlinGenerator].
const KotlinGenerator();

@override
void generate(StringSink sink, PigeonOptions options, Root root) {
KotlinOptions kotlinOptions =
options.kotlinOptions ?? const KotlinOptions();
kotlinOptions = kotlinOptions.merge(KotlinOptions(
copyrightHeader: options.copyrightHeader != null
? lineReader(options.copyrightHeader!)
: null));
generateKotlin(kotlinOptions, root, sink);
}

@override
IOSink? shouldGenerate(PigeonOptions options) => openSink(options.kotlinOut);

@override
List<Error> validate(PigeonOptions options, Root root) => <Error>[];
}

/// Calculates the name of the codec that will be generated for [api].
String _getCodecName(Api api) => '${api.name}Codec';

Expand Down
53 changes: 52 additions & 1 deletion packages/pigeon/lib/objc_generator.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:io';

import 'ast.dart';
import 'functional.dart';
import 'generator.dart';
import 'generator_tools.dart';
import 'pigeon_lib.dart' show Error, TaskQueueType;
import 'pigeon_lib.dart'
show Error, PigeonOptions, TaskQueueType, lineReader, openSink;

/// Documentation comment open symbol.
const String _docCommentPrefix = '///';
Expand Down Expand Up @@ -63,6 +67,53 @@ class ObjcOptions {
}
}

/// A [Generator] that generates Objective-C header code.
class ObjcHeaderGenerator implements Generator {
/// Constructor for [ObjcHeaderGenerator].
const ObjcHeaderGenerator();

@override
void generate(StringSink sink, PigeonOptions options, Root root) {
final ObjcOptions objcOptions = options.objcOptions ?? const ObjcOptions();
final ObjcOptions objcOptionsWithHeader = objcOptions.merge(ObjcOptions(
copyrightHeader: options.copyrightHeader != null
? lineReader(options.copyrightHeader!)
: null));
generateObjcHeader(objcOptionsWithHeader, root, sink);
}

@override
IOSink? shouldGenerate(PigeonOptions options) =>
openSink(options.objcHeaderOut);

@override
List<Error> validate(PigeonOptions options, Root root) =>
validateObjc(options.objcOptions!, root);
}

/// A [Generator] that generates Objective-C source code.
class ObjcSourceGenerator implements Generator {
/// Constructor for [ObjcSourceGenerator].
const ObjcSourceGenerator();

@override
void generate(StringSink sink, PigeonOptions options, Root root) {
final ObjcOptions objcOptions = options.objcOptions ?? const ObjcOptions();
final ObjcOptions objcOptionsWithHeader = objcOptions.merge(ObjcOptions(
copyrightHeader: options.copyrightHeader != null
? lineReader(options.copyrightHeader!)
: null));
generateObjcSource(objcOptionsWithHeader, root, sink);
}

@override
IOSink? shouldGenerate(PigeonOptions options) =>
openSink(options.objcSourceOut);

@override
List<Error> validate(PigeonOptions options, Root root) => <Error>[];
}

/// Calculates the ObjC class name, possibly prefixed.
String _className(String? prefix, String className) {
if (prefix != null) {
Expand Down
Loading