From 9b6fe68e8ca3e34a02913eedd686bea8894a9668 Mon Sep 17 00:00:00 2001 From: devmil Date: Tue, 16 Apr 2024 11:23:08 +0200 Subject: [PATCH 1/9] bump Flutter version to 3.19.5 update to new fvm format --- .fvm/fvm_config.json | 4 ---- .fvmrc | 4 ++++ .gitignore | 4 +++- 3 files changed, 7 insertions(+), 5 deletions(-) delete mode 100644 .fvm/fvm_config.json create mode 100644 .fvmrc diff --git a/.fvm/fvm_config.json b/.fvm/fvm_config.json deleted file mode 100644 index 2f5541f..0000000 --- a/.fvm/fvm_config.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "flutterSdkVersion": "3.16.9", - "flavors": {} -} diff --git a/.fvmrc b/.fvmrc new file mode 100644 index 0000000..db15b84 --- /dev/null +++ b/.fvmrc @@ -0,0 +1,4 @@ +{ + "flutter": "3.19.5", + "flavors": {} +} \ No newline at end of file diff --git a/.gitignore b/.gitignore index 67d2194..3d8adb3 100644 --- a/.gitignore +++ b/.gitignore @@ -19,7 +19,6 @@ doc/api/ *.js_ *.js.deps *.js.map -.fvm/flutter_sdk .idea .DS_Store .vscode/launch.json @@ -28,3 +27,6 @@ doc/api/ .vscode/ltex.hiddenFalsePositives.*.txt scripts/release_util/.vscode/launch.json coverage + +# FVM Version Cache +.fvm/ \ No newline at end of file From 125cbfd98175b9d67c60b97faf4f9bab5f356d3b Mon Sep 17 00:00:00 2001 From: devmil Date: Tue, 16 Apr 2024 11:34:27 +0200 Subject: [PATCH 2/9] reproduction test --- .../cli/extract_command_test.dart | 19 +++++++++ .../package_a/lib/types/class_a.dart | 1 - .../package_a/.gitignore | 10 +++++ .../package_a/CHANGELOG.md | 3 ++ .../package_a/README.md | 39 +++++++++++++++++++ .../package_a/analysis_options.yaml | 30 ++++++++++++++ .../package_a/lib/package_a.dart | 3 ++ .../package_a/lib/types/class_a.dart | 11 ++++++ .../package_a/lib/types/class_b.dart | 5 +++ .../package_a/pubspec.yaml | 16 ++++++++ 10 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 test/test_packages/missing_export_for_test/package_a/.gitignore create mode 100644 test/test_packages/missing_export_for_test/package_a/CHANGELOG.md create mode 100644 test/test_packages/missing_export_for_test/package_a/README.md create mode 100644 test/test_packages/missing_export_for_test/package_a/analysis_options.yaml create mode 100644 test/test_packages/missing_export_for_test/package_a/lib/package_a.dart create mode 100644 test/test_packages/missing_export_for_test/package_a/lib/types/class_a.dart create mode 100644 test/test_packages/missing_export_for_test/package_a/lib/types/class_b.dart create mode 100644 test/test_packages/missing_export_for_test/package_a/pubspec.yaml diff --git a/test/integration_tests/cli/extract_command_test.dart b/test/integration_tests/cli/extract_command_test.dart index 8ace982..8119c97 100644 --- a/test/integration_tests/cli/extract_command_test.dart +++ b/test/integration_tests/cli/extract_command_test.dart @@ -187,6 +187,25 @@ void main() { timeout: integrationTestTimeout, ); + test( + 'Handles `set-exit-on-missing-export` well if only @visibleForTesting usages are leaking', + () async { + final exitCode = await runner.run([ + 'extract', + '--input', + path.join( + 'test', + 'test_packages', + 'missing_export_for_test', + 'package_a', + ), + '--set-exit-on-missing-export', + ]); + expect(exitCode, 0); + }, + timeout: integrationTestTimeout, + ); + test( 'reports missing exports in extract result', () async { diff --git a/test/test_packages/missing_export/package_a/lib/types/class_a.dart b/test/test_packages/missing_export/package_a/lib/types/class_a.dart index 969656f..7ccbe8d 100644 --- a/test/test_packages/missing_export/package_a/lib/types/class_a.dart +++ b/test/test_packages/missing_export/package_a/lib/types/class_a.dart @@ -1,5 +1,4 @@ import 'package:meta/meta.dart'; -import 'package:package_b/package_b.dart'; import 'class_b.dart'; diff --git a/test/test_packages/missing_export_for_test/package_a/.gitignore b/test/test_packages/missing_export_for_test/package_a/.gitignore new file mode 100644 index 0000000..65c34dc --- /dev/null +++ b/test/test_packages/missing_export_for_test/package_a/.gitignore @@ -0,0 +1,10 @@ +# Files and directories created by pub. +.dart_tool/ +.packages + +# Conventional directory for build outputs. +build/ + +# Omit committing pubspec.lock for library packages; see +# https://dart.dev/guides/libraries/private-files#pubspeclock. +pubspec.lock diff --git a/test/test_packages/missing_export_for_test/package_a/CHANGELOG.md b/test/test_packages/missing_export_for_test/package_a/CHANGELOG.md new file mode 100644 index 0000000..effe43c --- /dev/null +++ b/test/test_packages/missing_export_for_test/package_a/CHANGELOG.md @@ -0,0 +1,3 @@ +## 1.0.0 + +- Initial version. diff --git a/test/test_packages/missing_export_for_test/package_a/README.md b/test/test_packages/missing_export_for_test/package_a/README.md new file mode 100644 index 0000000..8b55e73 --- /dev/null +++ b/test/test_packages/missing_export_for_test/package_a/README.md @@ -0,0 +1,39 @@ + + +TODO: Put a short description of the package here that helps potential users +know whether this package might be useful for them. + +## Features + +TODO: List what your package can do. Maybe include images, gifs, or videos. + +## Getting started + +TODO: List prerequisites and provide or point to information on how to +start using the package. + +## Usage + +TODO: Include short and useful examples for package users. Add longer examples +to `/example` folder. + +```dart +const like = 'sample'; +``` + +## Additional information + +TODO: Tell users more about the package: where to find more information, how to +contribute to the package, how to file issues, what response they can expect +from the package authors, and more. diff --git a/test/test_packages/missing_export_for_test/package_a/analysis_options.yaml b/test/test_packages/missing_export_for_test/package_a/analysis_options.yaml new file mode 100644 index 0000000..dee8927 --- /dev/null +++ b/test/test_packages/missing_export_for_test/package_a/analysis_options.yaml @@ -0,0 +1,30 @@ +# This file configures the static analysis results for your project (errors, +# warnings, and lints). +# +# This enables the 'recommended' set of lints from `package:lints`. +# This set helps identify many issues that may lead to problems when running +# or consuming Dart code, and enforces writing Dart using a single, idiomatic +# style and format. +# +# If you want a smaller set of lints you can change this to specify +# 'package:lints/core.yaml'. These are just the most critical lints +# (the recommended set includes the core lints). +# The core lints are also what is used by pub.dev for scoring packages. + +include: package:lints/recommended.yaml + +# Uncomment the following section to specify additional rules. + +# linter: +# rules: +# - camel_case_types + +# analyzer: +# exclude: +# - path/to/excluded/files/** + +# For more information about the core and recommended set of lints, see +# https://dart.dev/go/core-lints + +# For additional information about configuring this file, see +# https://dart.dev/guides/language/analysis-options diff --git a/test/test_packages/missing_export_for_test/package_a/lib/package_a.dart b/test/test_packages/missing_export_for_test/package_a/lib/package_a.dart new file mode 100644 index 0000000..95ac267 --- /dev/null +++ b/test/test_packages/missing_export_for_test/package_a/lib/package_a.dart @@ -0,0 +1,3 @@ +library package_a; + +export 'types/class_a.dart'; diff --git a/test/test_packages/missing_export_for_test/package_a/lib/types/class_a.dart b/test/test_packages/missing_export_for_test/package_a/lib/types/class_a.dart new file mode 100644 index 0000000..7b39610 --- /dev/null +++ b/test/test_packages/missing_export_for_test/package_a/lib/types/class_a.dart @@ -0,0 +1,11 @@ +import 'package:meta/meta.dart'; + +import 'class_b.dart'; + +@experimental +class ClassA { + ClassA(); + + @visibleForTesting + ClassB createClassB() => ClassB('someValue'); +} diff --git a/test/test_packages/missing_export_for_test/package_a/lib/types/class_b.dart b/test/test_packages/missing_export_for_test/package_a/lib/types/class_b.dart new file mode 100644 index 0000000..41299b8 --- /dev/null +++ b/test/test_packages/missing_export_for_test/package_a/lib/types/class_b.dart @@ -0,0 +1,5 @@ +class ClassB { + final String someProperty; + + ClassB(this.someProperty); +} diff --git a/test/test_packages/missing_export_for_test/package_a/pubspec.yaml b/test/test_packages/missing_export_for_test/package_a/pubspec.yaml new file mode 100644 index 0000000..5d3872a --- /dev/null +++ b/test/test_packages/missing_export_for_test/package_a/pubspec.yaml @@ -0,0 +1,16 @@ +name: package_a +description: A starting point for Dart libraries or applications. +version: 1.0.0 +publish_to: none +# homepage: https://www.example.com + +environment: + sdk: '>=2.18.4 <3.0.0' + +dependencies: + collection: ^1.17.0 + meta: ^1.8.0 + +dev_dependencies: + lints: ^2.0.0 + test: ^1.16.0 From a04e86a90f4f8da71922d643bbbd99d1397ff1fb Mon Sep 17 00:00:00 2001 From: devmil Date: Tue, 16 Apr 2024 14:06:10 +0200 Subject: [PATCH 3/9] track more type usage metadata referring element isVisibleForTesting makes the reproduction test pass adds another test for the grpc_dart case --- .../api_relevant_elements_collector.dart | 44 ++-- lib/src/analyze/package_api_analyzer.dart | 9 +- lib/src/cli/commands/extract_command.dart | 14 +- lib/src/model/interface_declaration.dart | 5 +- .../internal/internal_declaration_utils.dart | 32 +++ .../internal_interface_declaration.dart | 11 +- .../model/internal/internal_type_usage.dart | 31 +++ lib/src/model/model.dart | 1 + lib/src/model/package_api.dart | 10 +- lib/src/model/type_usage.dart | 28 ++- lib/src/model/type_usage.freezed.dart | 196 ++++++++++++++++++ lib/src/model/type_usage_kind.dart | 14 ++ lib/src/storage/package_api_storage.dart | 3 +- .../analyze/grpc_dart_test.dart | 36 ++++ 14 files changed, 395 insertions(+), 39 deletions(-) create mode 100644 lib/src/model/internal/internal_type_usage.dart create mode 100644 lib/src/model/type_usage.freezed.dart create mode 100644 lib/src/model/type_usage_kind.dart create mode 100644 test/integration_tests/analyze/grpc_dart_test.dart diff --git a/lib/src/analyze/api_relevant_elements_collector.dart b/lib/src/analyze/api_relevant_elements_collector.dart index 9b1c3e5..7b1dfdc 100644 --- a/lib/src/analyze/api_relevant_elements_collector.dart +++ b/lib/src/analyze/api_relevant_elements_collector.dart @@ -7,6 +7,7 @@ import 'package:analyzer/dart/element/visitor.dart'; import '../model/internal/internal_declaration_utils.dart'; import '../model/internal/internal_type_alias_declaration.dart'; +import '../model/internal/internal_type_usage.dart'; import '../model/model.dart'; import '../model/internal/internal_interface_declaration.dart'; import '../model/internal/internal_executable_declaration.dart'; @@ -66,7 +67,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { final List _executableDeclarations = []; final List _fieldDeclarations = []; final List _typeAliasDeclarations = []; - final Map> typeUsages = {}; + final Map> typeUsages = {}; final TypeHierarchy typeHierarchy; /// all found class declarations @@ -88,7 +89,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { final List privateElementExceptions; void _onTypeUsed(DartType type, Element referringElement, - {required TypeUsage typeUsage}) { + {required TypeUsageKind typeUsageKind}) { final directElement = type.element2; final directElementLibrary = directElement?.library; if (directElement == null || directElementLibrary == null) { @@ -97,7 +98,13 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { if (!typeUsages.containsKey(directElement.id)) { typeUsages[directElement.id] = {}; } - typeUsages[directElement.id]!.add(typeUsage); + typeUsages[directElement.id]!.add( + InternalTypeUsage.fromElement( + kind: typeUsageKind, + element: referringElement, + ), + ); + if (_collectedElementIds.contains(directElement.id)) { return; } @@ -136,7 +143,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { _onTypeUsed( ta, referringElement, - typeUsage: TypeUsage.hierarchy, + typeUsageKind: TypeUsageKind.hierarchy, ); } } @@ -146,7 +153,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { _onTypeUsed( aliasedType, referringElement, - typeUsage: TypeUsage.hierarchy, + typeUsageKind: TypeUsageKind.hierarchy, ); } } @@ -243,7 +250,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { )); for (final st in interfaceElement.allSupertypes) { if (!st.isDartCoreObject && !st.isDartCoreEnum) { - _onTypeUsed(st, interfaceElement, typeUsage: TypeUsage.hierarchy); + _onTypeUsed(st, interfaceElement, + typeUsageKind: TypeUsageKind.hierarchy); } } return true; @@ -289,9 +297,9 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { !element.isConst && !element.isPrivate && element.setter != null; - _onTypeUsed(element.type, element, typeUsage: TypeUsage.output); + _onTypeUsed(element.type, element, typeUsageKind: TypeUsageKind.output); if (canBeSet) { - _onTypeUsed(element.type, element, typeUsage: TypeUsage.input); + _onTypeUsed(element.type, element, typeUsageKind: TypeUsageKind.input); } } } @@ -314,9 +322,9 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { if (element.type.element2 != null) { bool canBeSet = !element.isFinal && !element.isConst && !element.isPrivate; - _onTypeUsed(element.type, element, typeUsage: TypeUsage.output); + _onTypeUsed(element.type, element, typeUsageKind: TypeUsageKind.output); if (canBeSet) { - _onTypeUsed(element.type, element, typeUsage: TypeUsage.input); + _onTypeUsed(element.type, element, typeUsageKind: TypeUsageKind.input); } } } @@ -331,7 +339,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { } // this includes method, function and constructor calls if (element.type.element2 != null) { - _onTypeUsed(element.type, element, typeUsage: TypeUsage.input); + _onTypeUsed(element.type, element, typeUsageKind: TypeUsageKind.input); } } @@ -351,7 +359,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { )); super.visitMethodElement(element); if (element.returnType.element2 != null) { - _onTypeUsed(element.returnType, element, typeUsage: TypeUsage.output); + _onTypeUsed(element.returnType, element, + typeUsageKind: TypeUsageKind.output); } } @@ -372,7 +381,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { )); super.visitFunctionElement(element); if (element.returnType.element2 != null) { - _onTypeUsed(element.returnType, element, typeUsage: TypeUsage.output); + _onTypeUsed(element.returnType, element, + typeUsageKind: TypeUsageKind.output); } } @@ -412,7 +422,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { )); super.visitTypeAliasElement(element); if (element.aliasedType.element2 != null) { - _onTypeUsed(element.aliasedType, element, typeUsage: TypeUsage.hierarchy); + _onTypeUsed(element.aliasedType, element, + typeUsageKind: TypeUsageKind.hierarchy); } } @@ -421,7 +432,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { _onVisitAnyElement(element); super.visitTypeParameterElement(element); if (element.bound?.element2 != null) { - _onTypeUsed(element.bound!, element, typeUsage: TypeUsage.hierarchy); + _onTypeUsed(element.bound!, element, + typeUsageKind: TypeUsageKind.hierarchy); } } @@ -445,7 +457,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor { )); if (element.extendedType.element2 != null) { _onTypeUsed(element.extendedType, element, - typeUsage: TypeUsage.hierarchy); + typeUsageKind: TypeUsageKind.hierarchy); } super.visitExtensionElement(element); diff --git a/lib/src/analyze/package_api_analyzer.dart b/lib/src/analyze/package_api_analyzer.dart index 5592051..cfe15bc 100644 --- a/lib/src/analyze/package_api_analyzer.dart +++ b/lib/src/analyze/package_api_analyzer.dart @@ -22,6 +22,7 @@ import 'package:pubspec_parse/pubspec_parse.dart'; import '../model/internal/internal_interface_declaration.dart'; import '../model/internal/internal_executable_declaration.dart'; import '../model/internal/internal_field_declaration.dart'; +import '../model/internal/internal_type_usage.dart'; import 'constraints/android_platform_constraints_helper.dart'; import 'constraints/ios_platform_contraints_helper.dart'; import 'dependencies/package_dependencies_helper.dart'; @@ -462,8 +463,10 @@ class PackageApiAnalyzer { final interfaceDeclaration = entry.interfaceDeclarations.single; // a merged element can be removed if it is not used as input or output and is unreachable from the outside bool isReachable = interfaceDeclaration.entryPoints?.isNotEmpty ?? false; - bool isInput = entry.typeUsages.contains(TypeUsage.input); - bool isOutput = entry.typeUsages.contains(TypeUsage.output); + bool isInput = + entry.typeUsages.any((tu) => tu.kind == TypeUsageKind.input); + bool isOutput = + entry.typeUsages.any((tu) => tu.kind == TypeUsageKind.output); if (!isReachable && !isInput && !isOutput) { collectedInterfaces.remove(mergedSuperTypeId); } @@ -529,7 +532,7 @@ class _InterfaceCollectionResult { List.empty(growable: true); final typeAliasDeclarations = List.empty(growable: true); - final typeUsages = {}; + final typeUsages = {}; } @freezed diff --git a/lib/src/cli/commands/extract_command.dart b/lib/src/cli/commands/extract_command.dart index ccce9de..6bb7932 100644 --- a/lib/src/cli/commands/extract_command.dart +++ b/lib/src/cli/commands/extract_command.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'package:args/command_runner.dart'; import 'package:dart_apitool/src/storage/storage.dart'; +import '../../model/model.dart'; import '../package_ref.dart'; import 'command_mixin.dart'; @@ -100,15 +101,20 @@ If not specified the extracted API will be printed to the console. stdout.writeln(jsonString); } - final declarationsWithoutEntryPoints = - packageApi.rootDeclarationsWithoutEntryPoints; + final declarationsWithoutEntryPointsOutsideTests = + packageApi.rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests; - if (declarationsWithoutEntryPoints.isNotEmpty) { + if (declarationsWithoutEntryPointsOutsideTests.isNotEmpty) { if (outFilePath == null) { stdout.writeln( 'The following declarations do not have an entry point (did you miss to export them?):'); - for (final declaration in declarationsWithoutEntryPoints) { + for (final declaration in declarationsWithoutEntryPointsOutsideTests) { stdout.writeln(' ${declaration.name}'); + if (declaration is InterfaceDeclaration) { + for (final typeUsage in declaration.typeUsages) { + stdout.writeln(' - ${typeUsage.referringElementName}'); + } + } } } if (doSetExitCodeOnMissingExport) { diff --git a/lib/src/model/interface_declaration.dart b/lib/src/model/interface_declaration.dart index 78b6e68..5ad0451 100644 --- a/lib/src/model/interface_declaration.dart +++ b/lib/src/model/interface_declaration.dart @@ -7,6 +7,7 @@ import 'declaration.dart'; import 'executable_declaration.dart'; import 'field_declaration.dart'; import 'type_usage.dart'; +import 'type_usage_kind.dart'; part 'interface_declaration.freezed.dart'; @@ -69,5 +70,7 @@ class InterfaceDeclaration with _$InterfaceDeclaration implements Declaration { /// determines if this interface is required (meaning: can be used in a type hierarchy by the consumer) bool get isRequired => - isAbstract && !isSealed && typeUsages.contains(TypeUsage.input); + isAbstract && + !isSealed && + typeUsages.any((tu) => tu.kind == TypeUsageKind.input); } diff --git a/lib/src/model/internal/internal_declaration_utils.dart b/lib/src/model/internal/internal_declaration_utils.dart index 0a49364..1b8dfb4 100644 --- a/lib/src/model/internal/internal_declaration_utils.dart +++ b/lib/src/model/internal/internal_declaration_utils.dart @@ -46,6 +46,10 @@ abstract class InternalDeclarationUtils { return result; } + static bool hasVisibleForTesting(Element element) { + return containsAnnotation(element, 'visibleForTesting'); + } + static bool hasExperimental(Element element) { return containsAnnotation(element, 'experimental'); } @@ -62,6 +66,34 @@ abstract class InternalDeclarationUtils { return ''; } + static String getFullQualifiedNameFor(Element element) { + final parts = []; + + /// stop at compilation unit level + adapt display name to show the relative path + if (element is CompilationUnitElement) { + Uri uri = element.source.uri; + String filePath; + try { + filePath = uri.toFilePath(); + final pathParts = path.split(uri.toFilePath()); + final libIndex = pathParts.lastIndexOf('lib'); + if (libIndex >= 0) { + pathParts.removeRange(0, libIndex); + } + return path.joinAll(pathParts); + } on UnsupportedError catch (_) { + return uri.toString(); + } + } + + if (element.enclosingElement != null) { + parts.add(getFullQualifiedNameFor(element.enclosingElement!)); + } + parts.add(element.displayName); + + return parts.join('::'); + } + static String? getNamespaceForElement( Element? referredElement, Element referringElement) { final referredElementLibrary = referredElement?.library; diff --git a/lib/src/model/internal/internal_interface_declaration.dart b/lib/src/model/internal/internal_interface_declaration.dart index 0920522..1e98258 100644 --- a/lib/src/model/internal/internal_interface_declaration.dart +++ b/lib/src/model/internal/internal_interface_declaration.dart @@ -7,6 +7,7 @@ import '../field_declaration.dart'; import '../type_usage.dart'; import 'internal_declaration.dart'; import 'internal_declaration_utils.dart'; +import 'internal_type_usage.dart'; /// Internal extension of [InterfaceDeclaration] that adds the [id] and [parentClassId] that is not stable between runs class InternalInterfaceDeclaration implements InternalDeclaration { @@ -112,7 +113,7 @@ class InternalInterfaceDeclaration implements InternalDeclaration { ); InterfaceDeclaration toInterfaceDeclaration( - {required Set typeUsages}) { + {required Set typeUsages}) { final namespacePrefix = namespace == null ? '' : '$namespace.'; return InterfaceDeclaration( name: '$namespacePrefix$name', @@ -125,7 +126,13 @@ class InternalInterfaceDeclaration implements InternalDeclaration { executableDeclarations: executableDeclarations, fieldDeclarations: fieldDeclarations, entryPoints: entryPoints, - typeUsages: typeUsages, + typeUsages: typeUsages + .map((itu) => TypeUsage( + kind: itu.kind, + referringElementName: itu.referringElementName, + isVisibleForTesting: itu.isVisibleForTesting, + )) + .toSet(), relativePath: relativePath, ); } diff --git a/lib/src/model/internal/internal_type_usage.dart b/lib/src/model/internal/internal_type_usage.dart new file mode 100644 index 0000000..073973f --- /dev/null +++ b/lib/src/model/internal/internal_type_usage.dart @@ -0,0 +1,31 @@ +import 'package:analyzer/dart/element/element.dart'; +import 'package:dart_apitool/src/model/internal/internal_declaration_utils.dart'; + +import '../model.dart'; + +/// internal representation of a type usage that extracts a qualified name of the referring element +class InternalTypeUsage { + TypeUsageKind kind; + + String referringElementName; + + bool isVisibleForTesting; + + InternalTypeUsage._({ + required this.kind, + required this.referringElementName, + required this.isVisibleForTesting, + }); + + /// creates an internal representation of a type usage + /// from the given [kind] and [element] + InternalTypeUsage.fromElement( + {required TypeUsageKind kind, required Element element}) + : this._( + kind: kind, + referringElementName: + InternalDeclarationUtils.getFullQualifiedNameFor(element), + isVisibleForTesting: + InternalDeclarationUtils.hasVisibleForTesting(element), + ); +} diff --git a/lib/src/model/model.dart b/lib/src/model/model.dart index e58dfa5..ff75865 100644 --- a/lib/src/model/model.dart +++ b/lib/src/model/model.dart @@ -10,3 +10,4 @@ export 'sdk_type.dart'; export 'type_alias_declaration.dart'; export 'type_hierarchy.dart'; export 'type_usage.dart'; +export 'type_usage_kind.dart'; diff --git a/lib/src/model/package_api.dart b/lib/src/model/package_api.dart index 6f1ee6a..854e92f 100644 --- a/lib/src/model/package_api.dart +++ b/lib/src/model/package_api.dart @@ -54,13 +54,19 @@ class PackageApi with _$PackageApi { }) = _PackageApi; /// returns all root level declarations of this package that don't have any entry points - Iterable get rootDeclarationsWithoutEntryPoints { + Iterable + get rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests { return [ - ...interfaceDeclarations.where((id) => id.entryPoints?.isEmpty ?? false), + ...interfaceDeclarations.where((id) => + (id.entryPoints?.isEmpty ?? false) && _isUsedOutsideTests(id)), ...executableDeclarations.where((ed) => ed.entryPoints?.isEmpty ?? false), ...typeAliasDeclarations .where((tad) => tad.entryPoints?.isEmpty ?? false), ...fieldDeclarations.where((fd) => fd.entryPoints?.isEmpty ?? false), ]; } + + static bool _isUsedOutsideTests(InterfaceDeclaration interfaceDeclaration) { + return interfaceDeclaration.typeUsages.any((tu) => !tu.isVisibleForTesting); + } } diff --git a/lib/src/model/type_usage.dart b/lib/src/model/type_usage.dart index d174694..190de64 100644 --- a/lib/src/model/type_usage.dart +++ b/lib/src/model/type_usage.dart @@ -1,14 +1,22 @@ -/// specifies how a type is used -enum TypeUsage { - /// the type is provided in the public API - provide, +import 'package:freezed_annotation/freezed_annotation.dart'; - /// the type is used to be passed into the API - input, +import 'type_usage_kind.dart'; - /// the type is used to be returned from the API - output, +part 'type_usage.freezed.dart'; - /// the type is used as a hierarchy element (base class, mixin, interface, type parameter ...) - hierarchy, +/// represents the usage of a type +@freezed +class TypeUsage with _$TypeUsage { + const TypeUsage._(); + + const factory TypeUsage({ + /// kind of usage + required TypeUsageKind kind, + + /// the name of the referring element + required String referringElementName, + + /// defines if the usage happened in a visibleForTesting context + required bool isVisibleForTesting, + }) = _TypeUsage; } diff --git a/lib/src/model/type_usage.freezed.dart b/lib/src/model/type_usage.freezed.dart new file mode 100644 index 0000000..0c1599a --- /dev/null +++ b/lib/src/model/type_usage.freezed.dart @@ -0,0 +1,196 @@ +// coverage:ignore-file +// GENERATED CODE - DO NOT MODIFY BY HAND +// ignore_for_file: type=lint +// ignore_for_file: unused_element, deprecated_member_use, deprecated_member_use_from_same_package, use_function_type_syntax_for_parameters, unnecessary_const, avoid_init_to_null, invalid_override_different_default_values_named, prefer_expression_function_bodies, annotate_overrides, invalid_annotation_target, unnecessary_question_mark + +part of 'type_usage.dart'; + +// ************************************************************************** +// FreezedGenerator +// ************************************************************************** + +T _$identity(T value) => value; + +final _privateConstructorUsedError = UnsupportedError( + 'It seems like you constructed your class using `MyClass._()`. This constructor is only meant to be used by freezed and you are not supposed to need it nor use it.\nPlease check the documentation here for more information: https://github.com/rrousselGit/freezed#adding-getters-and-methods-to-our-models'); + +/// @nodoc +mixin _$TypeUsage { + /// kind of usage + TypeUsageKind get kind => throw _privateConstructorUsedError; + + /// the name of the referring element + String get referringElementName => throw _privateConstructorUsedError; + + /// defines if the usage happened in a visibleForTesting context + bool get isVisibleForTesting => throw _privateConstructorUsedError; + + @JsonKey(ignore: true) + $TypeUsageCopyWith get copyWith => + throw _privateConstructorUsedError; +} + +/// @nodoc +abstract class $TypeUsageCopyWith<$Res> { + factory $TypeUsageCopyWith(TypeUsage value, $Res Function(TypeUsage) then) = + _$TypeUsageCopyWithImpl<$Res, TypeUsage>; + @useResult + $Res call( + {TypeUsageKind kind, + String referringElementName, + bool isVisibleForTesting}); +} + +/// @nodoc +class _$TypeUsageCopyWithImpl<$Res, $Val extends TypeUsage> + implements $TypeUsageCopyWith<$Res> { + _$TypeUsageCopyWithImpl(this._value, this._then); + + // ignore: unused_field + final $Val _value; + // ignore: unused_field + final $Res Function($Val) _then; + + @pragma('vm:prefer-inline') + @override + $Res call({ + Object? kind = null, + Object? referringElementName = null, + Object? isVisibleForTesting = null, + }) { + return _then(_value.copyWith( + kind: null == kind + ? _value.kind + : kind // ignore: cast_nullable_to_non_nullable + as TypeUsageKind, + referringElementName: null == referringElementName + ? _value.referringElementName + : referringElementName // ignore: cast_nullable_to_non_nullable + as String, + isVisibleForTesting: null == isVisibleForTesting + ? _value.isVisibleForTesting + : isVisibleForTesting // ignore: cast_nullable_to_non_nullable + as bool, + ) as $Val); + } +} + +/// @nodoc +abstract class _$$TypeUsageImplCopyWith<$Res> + implements $TypeUsageCopyWith<$Res> { + factory _$$TypeUsageImplCopyWith( + _$TypeUsageImpl value, $Res Function(_$TypeUsageImpl) then) = + __$$TypeUsageImplCopyWithImpl<$Res>; + @override + @useResult + $Res call( + {TypeUsageKind kind, + String referringElementName, + bool isVisibleForTesting}); +} + +/// @nodoc +class __$$TypeUsageImplCopyWithImpl<$Res> + extends _$TypeUsageCopyWithImpl<$Res, _$TypeUsageImpl> + implements _$$TypeUsageImplCopyWith<$Res> { + __$$TypeUsageImplCopyWithImpl( + _$TypeUsageImpl _value, $Res Function(_$TypeUsageImpl) _then) + : super(_value, _then); + + @pragma('vm:prefer-inline') + @override + $Res call({ + Object? kind = null, + Object? referringElementName = null, + Object? isVisibleForTesting = null, + }) { + return _then(_$TypeUsageImpl( + kind: null == kind + ? _value.kind + : kind // ignore: cast_nullable_to_non_nullable + as TypeUsageKind, + referringElementName: null == referringElementName + ? _value.referringElementName + : referringElementName // ignore: cast_nullable_to_non_nullable + as String, + isVisibleForTesting: null == isVisibleForTesting + ? _value.isVisibleForTesting + : isVisibleForTesting // ignore: cast_nullable_to_non_nullable + as bool, + )); + } +} + +/// @nodoc + +class _$TypeUsageImpl extends _TypeUsage { + const _$TypeUsageImpl( + {required this.kind, + required this.referringElementName, + required this.isVisibleForTesting}) + : super._(); + + /// kind of usage + @override + final TypeUsageKind kind; + + /// the name of the referring element + @override + final String referringElementName; + + /// defines if the usage happened in a visibleForTesting context + @override + final bool isVisibleForTesting; + + @override + String toString() { + return 'TypeUsage(kind: $kind, referringElementName: $referringElementName, isVisibleForTesting: $isVisibleForTesting)'; + } + + @override + bool operator ==(Object other) { + return identical(this, other) || + (other.runtimeType == runtimeType && + other is _$TypeUsageImpl && + (identical(other.kind, kind) || other.kind == kind) && + (identical(other.referringElementName, referringElementName) || + other.referringElementName == referringElementName) && + (identical(other.isVisibleForTesting, isVisibleForTesting) || + other.isVisibleForTesting == isVisibleForTesting)); + } + + @override + int get hashCode => + Object.hash(runtimeType, kind, referringElementName, isVisibleForTesting); + + @JsonKey(ignore: true) + @override + @pragma('vm:prefer-inline') + _$$TypeUsageImplCopyWith<_$TypeUsageImpl> get copyWith => + __$$TypeUsageImplCopyWithImpl<_$TypeUsageImpl>(this, _$identity); +} + +abstract class _TypeUsage extends TypeUsage { + const factory _TypeUsage( + {required final TypeUsageKind kind, + required final String referringElementName, + required final bool isVisibleForTesting}) = _$TypeUsageImpl; + const _TypeUsage._() : super._(); + + @override + + /// kind of usage + TypeUsageKind get kind; + @override + + /// the name of the referring element + String get referringElementName; + @override + + /// defines if the usage happened in a visibleForTesting context + bool get isVisibleForTesting; + @override + @JsonKey(ignore: true) + _$$TypeUsageImplCopyWith<_$TypeUsageImpl> get copyWith => + throw _privateConstructorUsedError; +} diff --git a/lib/src/model/type_usage_kind.dart b/lib/src/model/type_usage_kind.dart new file mode 100644 index 0000000..dfed402 --- /dev/null +++ b/lib/src/model/type_usage_kind.dart @@ -0,0 +1,14 @@ +/// specifies how a type is used +enum TypeUsageKind { + /// the type is provided in the public API + provide, + + /// the type is used to be passed into the API + input, + + /// the type is used to be returned from the API + output, + + /// the type is used as a hierarchy element (base class, mixin, interface, type parameter ...) + hierarchy, +} diff --git a/lib/src/storage/package_api_storage.dart b/lib/src/storage/package_api_storage.dart index 9d537e5..9b543b9 100644 --- a/lib/src/storage/package_api_storage.dart +++ b/lib/src/storage/package_api_storage.dart @@ -17,7 +17,8 @@ abstract class PackageApiStorage { return encoder.convert({ 'version': 3, 'packageApi': packageApiStorage.toJson(), - 'missingEntryPoints': packageApi.rootDeclarationsWithoutEntryPoints + 'missingEntryPoints': packageApi + .rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests .map((e) => e.name) .toList(), }); diff --git a/test/integration_tests/analyze/grpc_dart_test.dart b/test/integration_tests/analyze/grpc_dart_test.dart new file mode 100644 index 0000000..254043d --- /dev/null +++ b/test/integration_tests/analyze/grpc_dart_test.dart @@ -0,0 +1,36 @@ +import 'package:dart_apitool/api_tool.dart'; +import 'package:test/test.dart'; + +import '../helper/integration_test_helper.dart'; + +void main() { + group('analyzer gets analyzed correctly', () { + final packageGitUrl = 'https://github.com/grpc/grpc-dart.git'; + final packageGitRef = 'b05fafe77cffca15f56ca9bd33c5a51f6d2a7170'; + final retriever = GitPackageApiRetriever( + packageGitUrl, + packageGitRef, + ); + late PackageApi packageApi; + + setUpAll(() async { + packageApi = await retriever.retrieve(); + }); + + test('No missing exports are detected', () { + final rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests = + packageApi.rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests + .toList(); + expect( + rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests.length, 2); + expect( + rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests + .any((decl) => decl.name == '\$1.Duration'), + isTrue); + expect( + rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests + .any((decl) => decl.name == 'Any'), + isTrue); + }); + }); +} From 2158b146958931dc6282e6bf290327a2fd6637c7 Mon Sep 17 00:00:00 2001 From: devmil Date: Tue, 16 Apr 2024 14:11:39 +0200 Subject: [PATCH 4/9] adapt workflow to match new fvm config --- .github/workflows/ci.yml | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 33185c3..dc7c4ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,20 +23,14 @@ jobs: runs-on: ubuntu-latest outputs: - flutter-version: ${{ steps.step_extract_version.outputs.flutter-version }} + flutter-version: ${{ steps.fvm-config-action.outputs.FLUTTER_VERSION }} steps: - uses: actions/checkout@v3 - - uses: kuhnroyal/flutter-fvm-config-action@v1 - with: - path: '.fvm/fvm_config.json' - - - id: step_extract_version - run: | - FLUTTER_VERSION=${{ env.FLUTTER_VERSION }} - echo "flutter-version=${FLUTTER_VERSION}" >> $GITHUB_OUTPUT - + - uses: kuhnroyal/flutter-fvm-config-action@v2 + id: fvm-config-action + get_last_released_version: needs: check_label runs-on: ubuntu-latest From 6cf291c4881b4b2033b9e23674287f223ac43f74 Mon Sep 17 00:00:00 2001 From: devmil Date: Tue, 16 Apr 2024 14:17:03 +0200 Subject: [PATCH 5/9] fix analyzer issue --- lib/src/model/internal/internal_declaration_utils.dart | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/src/model/internal/internal_declaration_utils.dart b/lib/src/model/internal/internal_declaration_utils.dart index 1b8dfb4..bb9435f 100644 --- a/lib/src/model/internal/internal_declaration_utils.dart +++ b/lib/src/model/internal/internal_declaration_utils.dart @@ -72,9 +72,7 @@ abstract class InternalDeclarationUtils { /// stop at compilation unit level + adapt display name to show the relative path if (element is CompilationUnitElement) { Uri uri = element.source.uri; - String filePath; try { - filePath = uri.toFilePath(); final pathParts = path.split(uri.toFilePath()); final libIndex = pathParts.lastIndexOf('lib'); if (libIndex >= 0) { From 0f67559f4e02ae9b0e2e446fbb8e522c45b8e7b9 Mon Sep 17 00:00:00 2001 From: devmil Date: Tue, 16 Apr 2024 14:18:46 +0200 Subject: [PATCH 6/9] add CHANGELOG entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12e5f73..2618160 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Version 0.19.0 - introduces `force-use-flutter` option for all commands to force dart_apitool to use the `flutter` command. +- extend type usage tracking and fix situations in which types that are used in @visibleForTesting contexts were detected as not exported ## Version 0.18.0 - add missing export to json output for `extract` command From 6645b392c33b7b0a18457604a39e494a6b297d6b Mon Sep 17 00:00:00 2001 From: devmil Date: Tue, 16 Apr 2024 14:53:17 +0200 Subject: [PATCH 7/9] deal with package ref style compilation unit source URIs --- .../internal/internal_declaration_utils.dart | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/src/model/internal/internal_declaration_utils.dart b/lib/src/model/internal/internal_declaration_utils.dart index bb9435f..c9ff5d8 100644 --- a/lib/src/model/internal/internal_declaration_utils.dart +++ b/lib/src/model/internal/internal_declaration_utils.dart @@ -72,16 +72,23 @@ abstract class InternalDeclarationUtils { /// stop at compilation unit level + adapt display name to show the relative path if (element is CompilationUnitElement) { Uri uri = element.source.uri; - try { + if (uri.isScheme('file')) { final pathParts = path.split(uri.toFilePath()); final libIndex = pathParts.lastIndexOf('lib'); if (libIndex >= 0) { - pathParts.removeRange(0, libIndex); + pathParts.removeRange(0, libIndex + 1); } return path.joinAll(pathParts); - } on UnsupportedError catch (_) { - return uri.toString(); } + // in case we have a URI we just show the path segments. If we happen to find "src" then we only show the path after that + // this works as we only have usages in our own package + final uriPathSegments = [...uri.pathSegments]; + final srcIndex = uriPathSegments.lastIndexOf('src'); + if (srcIndex >= 0) { + uriPathSegments.removeRange(0, srcIndex + 1); + } + + return uriPathSegments.join('/'); } if (element.enclosingElement != null) { From 0d5f66024a03599d152a00211c074f2d784e8798 Mon Sep 17 00:00:00 2001 From: devmil Date: Wed, 17 Apr 2024 08:55:44 +0200 Subject: [PATCH 8/9] name tests after what they actually test --- test/integration_tests/analyze/grpc_dart_test.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/integration_tests/analyze/grpc_dart_test.dart b/test/integration_tests/analyze/grpc_dart_test.dart index 254043d..2118a21 100644 --- a/test/integration_tests/analyze/grpc_dart_test.dart +++ b/test/integration_tests/analyze/grpc_dart_test.dart @@ -17,7 +17,9 @@ void main() { packageApi = await retriever.retrieve(); }); - test('No missing exports are detected', () { + test( + "Detects 2 missing exports correctly (especially doesn't complain about 'ServerHandler')", + () { final rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests = packageApi.rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests .toList(); @@ -31,6 +33,10 @@ void main() { rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests .any((decl) => decl.name == 'Any'), isTrue); + expect( + rootDeclarationsWithoutEntryPointsAndVisibleOutsideTests + .any((decl) => decl.name == 'ServerHandler'), + isFalse); }); }); } From 4cbc4d55c6e28da226273ef1cb417543e4205711 Mon Sep 17 00:00:00 2001 From: devmil Date: Wed, 17 Apr 2024 08:59:03 +0200 Subject: [PATCH 9/9] make clear that the list under the missing exports are usages and only show the problematic ones --- lib/src/cli/commands/extract_command.dart | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/src/cli/commands/extract_command.dart b/lib/src/cli/commands/extract_command.dart index 6bb7932..3a17995 100644 --- a/lib/src/cli/commands/extract_command.dart +++ b/lib/src/cli/commands/extract_command.dart @@ -111,8 +111,14 @@ If not specified the extracted API will be printed to the console. for (final declaration in declarationsWithoutEntryPointsOutsideTests) { stdout.writeln(' ${declaration.name}'); if (declaration is InterfaceDeclaration) { - for (final typeUsage in declaration.typeUsages) { - stdout.writeln(' - ${typeUsage.referringElementName}'); + final filteredUsages = declaration.typeUsages + .where((tu) => !tu.isVisibleForTesting) + .toList(); + if (filteredUsages.isNotEmpty) { + stdout.writeln(' Usage(s):'); + for (final typeUsage in filteredUsages) { + stdout.writeln(' - ${typeUsage.referringElementName}'); + } } } }