diff --git a/packages/pigeon/CHANGELOG.md b/packages/pigeon/CHANGELOG.md index a87e4e99eee6..8e387e1da412 100644 --- a/packages/pigeon/CHANGELOG.md +++ b/packages/pigeon/CHANGELOG.md @@ -1,3 +1,7 @@ +## 27.1.1 + +* Fixes validation bug where empty classes without `sealed` keyword were not correctly flagged with an error. + ## 27.1.0 * [swift] Adds `CaseIterable` conformance to generated enums. diff --git a/packages/pigeon/lib/src/generator_tools.dart b/packages/pigeon/lib/src/generator_tools.dart index 4cc20cfd1cfa..dbc56d4823b8 100644 --- a/packages/pigeon/lib/src/generator_tools.dart +++ b/packages/pigeon/lib/src/generator_tools.dart @@ -15,7 +15,7 @@ import 'generator.dart'; /// The current version of pigeon. /// /// This must match the version in pubspec.yaml. -const String pigeonVersion = '27.1.0'; +const String pigeonVersion = '27.1.1'; /// Default plugin package name. const String defaultPluginPackageName = 'dev.flutter.pigeon'; diff --git a/packages/pigeon/lib/src/pigeon_lib_internal.dart b/packages/pigeon/lib/src/pigeon_lib_internal.dart index b9607e12f528..b4141fbb6c9e 100644 --- a/packages/pigeon/lib/src/pigeon_lib_internal.dart +++ b/packages/pigeon/lib/src/pigeon_lib_internal.dart @@ -619,6 +619,25 @@ List _validateAst(Root root, String source) { ), ); } + if (classDefinition.isSealed) { + if (classDefinition.fields.isNotEmpty) { + result.add( + Error(message: 'Sealed class: "${classDefinition.name}" must not contain fields.'), + ); + } + } + if (classDefinition.fields.isEmpty && !classDefinition.isSealed) { + result.add( + Error(message: 'Class: "${classDefinition.name}" must contain fields or be sealed.'), + ); + } + if (classDefinition.superClass != null) { + if (!classDefinition.superClass!.isSealed) { + result.add( + Error(message: 'Child class: "${classDefinition.name}" must extend a sealed class.'), + ); + } + } for (final NamedType field in getFieldsInSerializationOrder(classDefinition)) { final String? matchingPrefix = _findMatchingPrefixOrNull( field.name, @@ -644,26 +663,6 @@ List _validateAst(Root root, String source) { ), ); } - if (classDefinition.isSealed) { - if (classDefinition.fields.isNotEmpty) { - result.add( - Error( - message: 'Sealed class: "${classDefinition.name}" must not contain fields.', - lineNumber: _calculateLineNumberNullable(source, field.offset), - ), - ); - } - } - if (classDefinition.superClass != null) { - if (!classDefinition.superClass!.isSealed) { - result.add( - Error( - message: 'Child class: "${classDefinition.name}" must extend a sealed class.', - lineNumber: _calculateLineNumberNullable(source, field.offset), - ), - ); - } - } } } @@ -1805,22 +1804,21 @@ class RootBuilder extends dart_ast_visitor.RecursiveAstVisitor { lineNumber: calculateLineNumber(source, node.offset), ), ); - } else { - final dart_ast.TypeArgumentList? typeArguments = type.typeArguments; - final String name = node.fields.variables[0].name.lexeme; - final field = NamedType( - type: TypeDeclaration( - baseName: _getNamedTypeQualifiedName(type), - isNullable: type.question != null, - typeArguments: _typeAnnotationsToTypeArguments(typeArguments), - ), - name: name, - offset: node.offset, - defaultValue: _currentClassDefaultValues[name], - documentationComments: _documentationCommentsParser(node.documentationComment?.tokens), - ); - _currentClass!.fields.add(field); } + final dart_ast.TypeArgumentList? typeArguments = type.typeArguments; + final String name = node.fields.variables[0].name.lexeme; + final field = NamedType( + type: TypeDeclaration( + baseName: _getNamedTypeQualifiedName(type), + isNullable: type.question != null, + typeArguments: _typeAnnotationsToTypeArguments(typeArguments), + ), + name: name, + offset: node.offset, + defaultValue: _currentClassDefaultValues[name], + documentationComments: _documentationCommentsParser(node.documentationComment?.tokens), + ); + _currentClass!.fields.add(field); } else { _errors.add( Error( @@ -1969,23 +1967,22 @@ class RootBuilder extends dart_ast_visitor.RecursiveAstVisitor { lineNumber: calculateLineNumber(source, node.offset), ), ); - } else { - final dart_ast.TypeArgumentList? typeArguments = type.typeArguments; - (_currentApi as AstProxyApi?)!.fields.add( - ApiField( - type: TypeDeclaration( - baseName: _getNamedTypeQualifiedName(type), - isNullable: type.question != null, - typeArguments: _typeAnnotationsToTypeArguments(typeArguments), - ), - name: node.fields.variables[0].name.lexeme, - isAttached: _hasMetadata(node.metadata, 'attached') || isStatic, - isStatic: isStatic, - offset: node.offset, - documentationComments: _documentationCommentsParser(node.documentationComment?.tokens), - ), - ); } + final dart_ast.TypeArgumentList? typeArguments = type.typeArguments; + (_currentApi as AstProxyApi?)!.fields.add( + ApiField( + type: TypeDeclaration( + baseName: _getNamedTypeQualifiedName(type), + isNullable: type.question != null, + typeArguments: _typeAnnotationsToTypeArguments(typeArguments), + ), + name: node.fields.variables[0].name.lexeme, + isAttached: _hasMetadata(node.metadata, 'attached') || isStatic, + isStatic: isStatic, + offset: node.offset, + documentationComments: _documentationCommentsParser(node.documentationComment?.tokens), + ), + ); } } } diff --git a/packages/pigeon/pubspec.yaml b/packages/pigeon/pubspec.yaml index 65afaca2890f..bf3b9563ca90 100644 --- a/packages/pigeon/pubspec.yaml +++ b/packages/pigeon/pubspec.yaml @@ -2,7 +2,7 @@ name: pigeon description: Code generator tool to make communication between Flutter and the host platform type-safe and easier. repository: https://github.com/flutter/packages/tree/main/packages/pigeon issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+pigeon%22 -version: 27.1.0 # This must match the version in lib/src/generator_tools.dart +version: 27.1.1 # This must match the version in lib/src/generator_tools.dart environment: sdk: ^3.10.0 diff --git a/packages/pigeon/test/pigeon_lib_test.dart b/packages/pigeon/test/pigeon_lib_test.dart index 7c319672e81d..df0f12d29486 100644 --- a/packages/pigeon/test/pigeon_lib_test.dart +++ b/packages/pigeon/test/pigeon_lib_test.dart @@ -215,6 +215,23 @@ abstract class Api { expect(results.errors[0].message, contains('dynamic')); }); + test('empty class error', () { + const source = ''' +class EmptyClass {} + +@HostApi() +abstract class Api { + void foo(EmptyClass empty); +} +'''; + final ParseResults results = parseSource(source); + expect(results.errors, hasLength(1)); + expect( + results.errors[0].message, + contains('Class: "EmptyClass" must contain fields or be sealed.'), + ); + }); + test('Only allow one api annotation', () { const source = ''' @HostApi() @@ -1703,7 +1720,9 @@ abstract class EventChannelApi { group('sealed inheritance validation', () { test('super class must be sealed', () { const code = ''' -class DataClass {} +class DataClass { + int? someField; +} class ChildClass extends DataClass { ChildClass(this.input); int input;