Skip to content
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.

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

Respect visible for testing in api leak check + improve type usage tracking #180

Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 0 additions & 4 deletions .fvm/fvm_config.json

This file was deleted.

4 changes: 4 additions & 0 deletions .fvmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"flutter": "3.19.5",
"flavors": {}
}
14 changes: 4 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ doc/api/
*.js_
*.js.deps
*.js.map
.fvm/flutter_sdk
.idea
.DS_Store
.vscode/launch.json
Expand All @@ -28,3 +27,6 @@ doc/api/
.vscode/ltex.hiddenFalsePositives.*.txt
scripts/release_util/.vscode/launch.json
coverage

# FVM Version Cache
.fvm/
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 28 additions & 16 deletions lib/src/analyze/api_relevant_elements_collector.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -66,7 +67,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
final List<InternalExecutableDeclaration> _executableDeclarations = [];
final List<InternalFieldDeclaration> _fieldDeclarations = [];
final List<InternalTypeAliasDeclaration> _typeAliasDeclarations = [];
final Map<int, Set<TypeUsage>> typeUsages = {};
final Map<int, Set<InternalTypeUsage>> typeUsages = {};
final TypeHierarchy typeHierarchy;

/// all found class declarations
Expand All @@ -88,7 +89,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
final List<int> 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) {
Expand All @@ -97,7 +98,13 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
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;
}
Expand Down Expand Up @@ -136,7 +143,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
_onTypeUsed(
ta,
referringElement,
typeUsage: TypeUsage.hierarchy,
typeUsageKind: TypeUsageKind.hierarchy,
);
}
}
Expand All @@ -146,7 +153,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
_onTypeUsed(
aliasedType,
referringElement,
typeUsage: TypeUsage.hierarchy,
typeUsageKind: TypeUsageKind.hierarchy,
);
}
}
Expand Down Expand Up @@ -243,7 +250,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
));
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;
Expand Down Expand Up @@ -289,9 +297,9 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
!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);
}
}
}
Expand All @@ -314,9 +322,9 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
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);
}
}
}
Expand All @@ -331,7 +339,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
}
// 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);
}
}

Expand All @@ -351,7 +359,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
));
super.visitMethodElement(element);
if (element.returnType.element2 != null) {
_onTypeUsed(element.returnType, element, typeUsage: TypeUsage.output);
_onTypeUsed(element.returnType, element,
typeUsageKind: TypeUsageKind.output);
}
}

Expand All @@ -372,7 +381,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
));
super.visitFunctionElement(element);
if (element.returnType.element2 != null) {
_onTypeUsed(element.returnType, element, typeUsage: TypeUsage.output);
_onTypeUsed(element.returnType, element,
typeUsageKind: TypeUsageKind.output);
}
}

Expand Down Expand Up @@ -412,7 +422,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
));
super.visitTypeAliasElement(element);
if (element.aliasedType.element2 != null) {
_onTypeUsed(element.aliasedType, element, typeUsage: TypeUsage.hierarchy);
_onTypeUsed(element.aliasedType, element,
typeUsageKind: TypeUsageKind.hierarchy);
}
}

Expand All @@ -421,7 +432,8 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
_onVisitAnyElement(element);
super.visitTypeParameterElement(element);
if (element.bound?.element2 != null) {
_onTypeUsed(element.bound!, element, typeUsage: TypeUsage.hierarchy);
_onTypeUsed(element.bound!, element,
typeUsageKind: TypeUsageKind.hierarchy);
}
}

Expand All @@ -445,7 +457,7 @@ class APIRelevantElementsCollector extends RecursiveElementVisitor<void> {
));
if (element.extendedType.element2 != null) {
_onTypeUsed(element.extendedType, element,
typeUsage: TypeUsage.hierarchy);
typeUsageKind: TypeUsageKind.hierarchy);
}

super.visitExtensionElement(element);
Expand Down
9 changes: 6 additions & 3 deletions lib/src/analyze/package_api_analyzer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -529,7 +532,7 @@ class _InterfaceCollectionResult {
List<InternalFieldDeclaration>.empty(growable: true);
final typeAliasDeclarations =
List<InternalTypeAliasDeclaration>.empty(growable: true);
final typeUsages = <TypeUsage>{};
final typeUsages = <InternalTypeUsage>{};
}

@freezed
Expand Down
20 changes: 16 additions & 4 deletions lib/src/cli/commands/extract_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -100,15 +101,26 @@ 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) {
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}');
}
}
}
}
}
if (doSetExitCodeOnMissingExport) {
Expand Down
5 changes: 4 additions & 1 deletion lib/src/model/interface_declaration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
}
37 changes: 37 additions & 0 deletions lib/src/model/internal/internal_declaration_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand All @@ -62,6 +66,39 @@ abstract class InternalDeclarationUtils {
return '';
}

static String getFullQualifiedNameFor(Element element) {
final parts = <String>[];

/// stop at compilation unit level + adapt display name to show the relative path
if (element is CompilationUnitElement) {
Uri uri = element.source.uri;
if (uri.isScheme('file')) {
final pathParts = path.split(uri.toFilePath());
final libIndex = pathParts.lastIndexOf('lib');
if (libIndex >= 0) {
pathParts.removeRange(0, libIndex + 1);
}
return path.joinAll(pathParts);
}
// 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) {
parts.add(getFullQualifiedNameFor(element.enclosingElement!));
}
parts.add(element.displayName);

return parts.join('::');
}

static String? getNamespaceForElement(
Element? referredElement, Element referringElement) {
final referredElementLibrary = referredElement?.library;
Expand Down
11 changes: 9 additions & 2 deletions lib/src/model/internal/internal_interface_declaration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -112,7 +113,7 @@ class InternalInterfaceDeclaration implements InternalDeclaration {
);

InterfaceDeclaration toInterfaceDeclaration(
{required Set<TypeUsage> typeUsages}) {
{required Set<InternalTypeUsage> typeUsages}) {
final namespacePrefix = namespace == null ? '' : '$namespace.';
return InterfaceDeclaration(
name: '$namespacePrefix$name',
Expand All @@ -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,
);
}
Expand Down
Loading
Loading