Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
171 changes: 165 additions & 6 deletions packages/pigeon/lib/src/pigeon_lib.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import 'package:analyzer/dart/analysis/analysis_context_collection.dart'
show AnalysisContextCollection;
import 'package:analyzer/dart/analysis/results.dart' show ParsedUnitResult;
import 'package:analyzer/dart/analysis/session.dart' show AnalysisSession;
import 'package:analyzer/dart/analysis/utilities.dart' show parseString;
import 'package:analyzer/dart/ast/ast.dart' as dart_ast;
import 'package:analyzer/diagnostic/diagnostic.dart' show Diagnostic;
import 'package:args/args.dart';
Expand Down Expand Up @@ -454,22 +455,69 @@ class Pigeon {
/// [sdkPath] for specifying the Dart SDK path for
/// [AnalysisContextCollection].
ParseResults parseFile(String inputPath, {String? sdkPath}) {
final includedPaths = <String>[path.absolute(path.normalize(inputPath))];
final String normalizedInputPath = path.absolute(path.normalize(inputPath));
final String? mainContent = _readFileOrNull(normalizedInputPath);
if (mainContent == null) {
return ParseResults(
root: Root.makeEmpty(),
errors: <Error>[
Error(
message: 'File $normalizedInputPath does not exist',
filename: normalizedInputPath,
),
],
pigeonOptions: null,
);
}
final dart_ast.CompilationUnit mainUnit = parseString(
content: mainContent,
path: normalizedInputPath,
throwIfDiagnostics: false,
).unit;
final List<String> parts = _getPartPaths(
mainUnit.directives,
sourcePath: normalizedInputPath,
);

final includedPaths = <String>[normalizedInputPath, ...parts];

final collection = AnalysisContextCollection(
includedPaths: includedPaths,
sdkPath: sdkPath,
);

final compilationErrors = <Error>[];
final rootBuilder = RootBuilder(File(inputPath).readAsStringSync());
final String? rootInputString = _getInputString(
inputPath: normalizedInputPath,
inputContent: mainContent,
inputUnit: mainUnit,
partPaths: parts,
);
if (rootInputString == null) {
return ParseResults(
root: Root.makeEmpty(),
errors: <Error>[
Error(
message:
'Part file referenced by $normalizedInputPath does not exist',
filename: normalizedInputPath,
),
],
pigeonOptions: null,
);
}
final rootBuilder = RootBuilder(rootInputString);
final dart_ast.CompilationUnit mergedUnit = parseString(
content: rootInputString,
path: normalizedInputPath,
throwIfDiagnostics: false,
).unit;
mergedUnit.accept(rootBuilder);
Comment on lines +485 to +491
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Merging the contents of the main file and its parts into a single rootInputString causes a regression in error reporting. Any errors identified by RootBuilder (e.g., invalid types, unsupported constructs) will report line numbers relative to this concatenated string and attribute them to the main file path, making it difficult for users to locate the actual issue in their source files. Consider a strategy that preserves the mapping to original files or avoids string concatenation by visiting multiple CompilationUnits while providing the correct source context for each.

for (final AnalysisContext context in collection.contexts) {
for (final String path in context.contextRoot.analyzedFiles()) {
final AnalysisSession session = context.currentSession;
final result = session.getParsedUnit(path) as ParsedUnitResult;
if (result.diagnostics.isEmpty) {
final dart_ast.CompilationUnit unit = result.unit;
unit.accept(rootBuilder);
} else {
if (result.diagnostics.isNotEmpty) {
for (final Diagnostic diagnostic in result.diagnostics) {
compilationErrors.add(
Error(
Expand Down Expand Up @@ -497,6 +545,117 @@ class Pigeon {
}
}

String? _readFileOrNull(String filePath) {
final file = File(filePath);
if (!file.existsSync()) {
return null;
}
return file.readAsStringSync();
}

List<String> _getPartPaths(
Iterable<dart_ast.Directive> directives, {
required String sourcePath,
}) {
final parts = <String>[];
for (final directive in directives) {
if (directive is dart_ast.PartDirective) {
final String? uri = directive.uri.stringValue;
if (uri != null) {
parts.add(
path.absolute(
path.normalize(path.join(path.dirname(sourcePath), uri)),
),
);
}
}
}
return parts;
}
Comment on lines +532 to +550
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The current implementation of _getPartPaths only retrieves part files referenced directly in the main Pigeon file. It does not support nested part files (i.e., a part file that itself contains part directives), which is valid Dart. To fully support the part mechanism, this should recursively resolve part paths.


({String body, List<String> imports}) _stripPartsAndCollectImports({
required String sourceContent,
required dart_ast.CompilationUnit unit,
required bool collectImports,
}) {
final imports = <String>[];
final removalRanges = <({int start, int end})>[];
for (final dart_ast.Directive directive in unit.directives) {
if (directive is dart_ast.ImportDirective) {
if (collectImports) {
imports.add(
sourceContent
.substring(directive.offset, directive.end)
.trimRight(),
);
}
removalRanges.add((start: directive.offset, end: directive.end));
} else if (directive is dart_ast.PartDirective ||
directive is dart_ast.PartOfDirective) {
removalRanges.add((start: directive.offset, end: directive.end));
}
Comment on lines +569 to +573
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The _stripPartsAndCollectImports function does not handle LibraryDirectives. If the main Pigeon file contains a library directive, it will remain in the body while imports are moved to the top in _getInputString. This results in invalid Dart code (as library must precede import), which may cause the subsequent parseString call or RootBuilder to behave unexpectedly. Consider stripping LibraryDirective as well.

      } else if (directive is dart_ast.PartDirective ||
          directive is dart_ast.PartOfDirective ||
          directive is dart_ast.LibraryDirective) {
        removalRanges.add((start: directive.offset, end: directive.end));
      }

}

final body = StringBuffer();
var start = 0;
for (final range in removalRanges) {
body.write(sourceContent.substring(start, range.start));
start = range.end;
}
body.write(sourceContent.substring(start));
return (body: body.toString().trim(), imports: imports);
}

String? _getInputString({
required String inputPath,
required String inputContent,
required dart_ast.CompilationUnit inputUnit,
required List<String> partPaths,
}) {
final ({String body, List<String> imports}) mainResult =
_stripPartsAndCollectImports(
sourceContent: inputContent,
unit: inputUnit,
collectImports: true,
);
final List<String> imports = mainResult.imports;

final partBodies = <String>[];
for (final partPath in partPaths) {
final String? partContent = _readFileOrNull(partPath);
if (partContent == null) {
return null;
}
final dart_ast.CompilationUnit partUnit = parseString(
content: partContent,
path: partPath,
throwIfDiagnostics: false,
).unit;
final ({String body, List<String> imports}) partResult =
_stripPartsAndCollectImports(
sourceContent: partContent,
unit: partUnit,
collectImports: false,
);
partBodies.add(partResult.body);
}

final output = StringBuffer();
if (imports.isNotEmpty) {
output.writeln(imports.join('\n'));
output.writeln();
}
output.writeln(mainResult.body);
for (final partBody in partBodies) {
if (partBody.isNotEmpty) {
output.writeln();
output.writeln();
output.write(partBody);
}
}
return output.toString().trimRight();
}

/// String that describes how the tool is used.
static String get usage {
return '''
Expand Down
79 changes: 79 additions & 0 deletions packages/pigeon/test/pigeon_lib_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,24 @@ void main() {
return results!;
}

ParseResults parseSourceWithParts({
required String mainSource,
required Map<String, String> partSources,
}) {
final Pigeon dartle = Pigeon.setup();
final Directory dir = Directory.systemTemp.createTempSync();
try {
for (final MapEntry<String, String> part in partSources.entries) {
File('${dir.path}/${part.key}').writeAsStringSync(part.value);
}
final mainFile = File('${dir.path}/source.dart')
..writeAsStringSync(mainSource);
return dartle.parseFile(mainFile.path);
} finally {
dir.deleteSync(recursive: true);
}
}

test('parse args - input', () {
final PigeonOptions opts = Pigeon.parseArgs(<String>[
'--input',
Expand Down Expand Up @@ -249,6 +267,67 @@ abstract class Api1 {
expect(unused?.fields[0].type.isNullable, isTrue);
});

test('parse source split across multiple part files', () {
const mainSource = '''
part 'shared_classes.dart';
part 'api.dart';
''';
const sharedClassesPart = '''
part of 'source.dart';

class Input1 {
String? input;
}

class Output1 {
String? output;
}
''';
const apiPart = '''
part of 'source.dart';

@HostApi()
abstract class Api1 {
Output1 doit(Input1 input);
}
''';
final ParseResults parseResult = parseSourceWithParts(
mainSource: mainSource,
partSources: <String, String>{
'shared_classes.dart': sharedClassesPart,
'api.dart': apiPart,
},
);

expect(parseResult.errors, isEmpty);
expect(parseResult.root.apis, hasLength(1));
expect(parseResult.root.apis[0].name, equals('Api1'));
expect(parseResult.root.apis[0].methods, hasLength(1));
expect(parseResult.root.apis[0].methods[0].name, equals('doit'));
expect(parseResult.root.apis[0].methods[0].returnType.baseName, 'Output1');
expect(parseResult.root.apis[0].methods[0].parameters, hasLength(1));
expect(
parseResult.root.apis[0].methods[0].parameters[0].type.baseName,
equals('Input1'),
);
expect(
parseResult.root.classes.map(
(Class classDefinition) => classDefinition.name,
),
containsAll(<String>['Input1', 'Output1']),
);
});

test('missing part file returns parse error', () {
const source = '''
part 'missing.dart';
''';
final ParseResults parseResult = parseSource(source);
expect(parseResult.root.apis, isEmpty);
expect(parseResult.root.classes, isEmpty);
expect(parseResult.errors, isNotEmpty);
});

test('invalid datatype', () {
const source = '''
class InvalidDatatype {
Expand Down