From 862d83373be165ef01195aaccae18e5a1e22b053 Mon Sep 17 00:00:00 2001 From: Brian Wilkerson Date: Fri, 19 Mar 2021 11:28:06 -0700 Subject: [PATCH 1/3] Implement the lint library_private_types_in_public_api --- example/all.yaml | 1 + lib/src/rules.dart | 2 + .../library_private_types_in_public_api.dart | 246 ++++++++++++++++++ .../library_private_types_in_public_api.dart | 123 +++++++++ 4 files changed, 372 insertions(+) create mode 100644 lib/src/rules/library_private_types_in_public_api.dart create mode 100644 test/rules/library_private_types_in_public_api.dart diff --git a/example/all.yaml b/example/all.yaml index 0fbcb8fc2..c927411ad 100644 --- a/example/all.yaml +++ b/example/all.yaml @@ -77,6 +77,7 @@ linter: - leading_newlines_in_multiline_strings - library_names - library_prefixes + - library_private_types_in_public_api - lines_longer_than_80_chars - list_remove_unrelated_type - literal_only_boolean_expressions diff --git a/lib/src/rules.dart b/lib/src/rules.dart index 95e487bdd..83e416fa3 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -79,6 +79,7 @@ import 'rules/join_return_with_assignment.dart'; import 'rules/leading_newlines_in_multiline_strings.dart'; import 'rules/library_names.dart'; import 'rules/library_prefixes.dart'; +import 'rules/library_private_types_in_public_api.dart'; import 'rules/lines_longer_than_80_chars.dart'; import 'rules/list_remove_unrelated_type.dart'; import 'rules/literal_only_boolean_expressions.dart'; @@ -269,6 +270,7 @@ void registerLintRules({bool inTestMode = false}) { ..register(LeadingNewlinesInMultilineStrings()) ..register(LibraryNames()) ..register(LibraryPrefixes()) + ..register(LibraryPrivateTypeInPublicAPI()) ..register(LinesLongerThan80Chars()) ..register(ListRemoveUnrelatedType()) ..register(LiteralOnlyBooleanExpressions()) diff --git a/lib/src/rules/library_private_types_in_public_api.dart b/lib/src/rules/library_private_types_in_public_api.dart new file mode 100644 index 000000000..73d62370c --- /dev/null +++ b/lib/src/rules/library_private_types_in_public_api.dart @@ -0,0 +1,246 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; + +import '../analyzer.dart'; + +const _desc = r'Avoid using private types in public APIs.'; + +const _details = r''' + +**AVOID** using private types in public APIs. + +**GOOD:** +``` +f(String s) { ... } +``` + +**BAD:** +``` +f(_Private p) { ... } +class _Private {} +``` + +'''; + +class LibraryPrivateTypeInPublicAPI extends LintRule implements NodeLintRule { + LibraryPrivateTypeInPublicAPI() + : super( + name: 'library_private_types_in_public_api', + description: _desc, + details: _details, + group: Group.style); + + @override + void registerNodeProcessors( + NodeLintRegistry registry, LinterContext context) { + final visitor = Visitor(this); + registry.addCompilationUnit(this, visitor); + } +} + +class Validator extends SimpleAstVisitor { + LintRule rule; + + Validator(this.rule); + + @override + void visitClassDeclaration(ClassDeclaration node) { + if (Identifier.isPrivateName(node.name.name)) { + return; + } + node.typeParameters?.accept(this); + node.members.accept(this); + } + + @override + void visitClassTypeAlias(ClassTypeAlias node) { + if (Identifier.isPrivateName(node.name.name)) { + return; + } + node.superclass.accept(this); + node.typeParameters?.accept(this); + } + + @override + void visitConstructorDeclaration(ConstructorDeclaration node) { + var name = node.name; + if (name != null && Identifier.isPrivateName(name.name)) { + return; + } + node.parameters.accept(this); + } + + @override + void visitDefaultFormalParameter(DefaultFormalParameter node) { + node.parameter.accept(this); + } + + @override + void visitExtensionDeclaration(ExtensionDeclaration node) { + var name = node.name; + if (name == null || Identifier.isPrivateName(name.name)) { + return; + } + node.extendedType.accept(this); + node.typeParameters?.accept(this); + node.members.accept(this); + } + + @override + void visitFieldDeclaration(FieldDeclaration node) { + if (node.fields.variables + .any((field) => !Identifier.isPrivateName(field.name.name))) { + node.fields.type?.accept(this); + } + } + + @override + void visitFieldFormalParameter(FieldFormalParameter node) { + if (node.isNamed && Identifier.isPrivateName(node.identifier.name)) { + return; + } + node.type?.accept(this); + } + + @override + void visitFormalParameterList(FormalParameterList node) { + node.parameters.accept(this); + } + + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + if (Identifier.isPrivateName(node.name.name)) { + return; + } + node.returnType?.accept(this); + node.functionExpression.typeParameters?.accept(this); + node.functionExpression.parameters?.accept(this); + } + + @override + void visitFunctionTypeAlias(FunctionTypeAlias node) { + if (Identifier.isPrivateName(node.name.name)) { + return; + } + node.returnType?.accept(this); + node.typeParameters?.accept(this); + node.parameters.accept(this); + } + + @override + void visitFunctionTypedFormalParameter(FunctionTypedFormalParameter node) { + if (node.isNamed && Identifier.isPrivateName(node.identifier.name)) { + return; + } + node.returnType?.accept(this); + node.typeParameters?.accept(this); + node.parameters.accept(this); + } + + @override + void visitGenericFunctionType(GenericFunctionType node) { + node.returnType?.accept(this); + node.typeParameters?.accept(this); + node.parameters.accept(this); + } + + @override + void visitGenericTypeAlias(GenericTypeAlias node) { + if (Identifier.isPrivateName(node.name.name)) { + return; + } + node.typeParameters?.accept(this); + node.functionType?.accept(this); + } + + @override + void visitMethodDeclaration(MethodDeclaration node) { + if (Identifier.isPrivateName(node.name.name)) { + return; + } + node.returnType?.accept(this); + node.typeParameters?.accept(this); + node.parameters?.accept(this); + } + + @override + void visitMixinDeclaration(MixinDeclaration node) { + if (Identifier.isPrivateName(node.name.name)) { + return; + } + node.onClause?.superclassConstraints.accept(this); + node.typeParameters?.accept(this); + node.members.accept(this); + } + + @override + void visitSimpleFormalParameter(SimpleFormalParameter node) { + var name = node.identifier; + if (name != null && node.isNamed && Identifier.isPrivateName(name.name)) { + return; + } + node.type?.accept(this); + } + + @override + void visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) { + if (node.variables.variables + .any((field) => !Identifier.isPrivateName(field.name.name))) { + node.variables.type?.accept(this); + } + } + + @override + void visitTypeArgumentList(TypeArgumentList node) { + node.arguments.accept(this); + } + + @override + void visitTypeName(TypeName node) { + var element = node.name.staticElement; + if (element != null && isPrivate(element)) { + rule.reportLint(node.name); + } + node.typeArguments?.accept(this); + } + + @override + void visitTypeParameter(TypeParameter node) { + node.bound?.accept(this); + } + + @override + void visitTypeParameterList(TypeParameterList node) { + node.typeParameters.accept(this); + } + + /// Return `true` if the given [element] is private or is defined in a private + /// library. + static bool isPrivate(Element element) { + var name = element.name; + return name != null && Identifier.isPrivateName(name); + } +} + +class Visitor extends SimpleAstVisitor { + LintRule rule; + + Visitor(this.rule); + + @override + void visitCompilationUnit(CompilationUnit node) { + if (!Validator.isPrivate(node.declaredElement!)) { + var validator = Validator(rule); + try { + node.declarations.accept(validator); + } catch (e, st) { + DateTime.now(); + } + } + } +} diff --git a/test/rules/library_private_types_in_public_api.dart b/test/rules/library_private_types_in_public_api.dart new file mode 100644 index 000000000..ae01ed488 --- /dev/null +++ b/test/rules/library_private_types_in_public_api.dart @@ -0,0 +1,123 @@ +// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// test w/ `pub run test -N library_private_types_in_public_api` + +// Top level functions + +String f1(int i) => ''; +_Private1 f2(int i) => _Private1.fromInt(0); // LINT +String f3(_Private1 p) => ''; // LINT +_Private1 _f4(_Private1 p) => _Private1.fromInt(0); + +// Top level variables + +String v1 = ''; +_Private1? v2; // LINT +_Private2? v3; // LINT +List<_Private1> v4 = []; // LINT +_Private1 _v5 = _Private1.fromInt(0); + +// Top level getters + +String get g1 => ''; +_Private1 get g2 => _Private1.fromInt(0); // LINT +_Private1 get _g3 => _Private1.fromInt(0); + +// Top level setters + +set s1(int i) {} +set s2(_Private1 i) {} // LINT +set _s3(_Private1 i) {} + +// Type aliases + +typedef _Private1 F1(); // LINT +typedef void F2(_Private1 p); // LINT +typedef F3 = _Private1 Function(); // LINT +typedef F4 = void Function(_Private1); // LINT +typedef String F5(); +typedef F6 = void Function(int); + +// Classes + +class Public1 {} + +class Public2 extends Public4<_Private1> {} + +class Public3 extends Object with _Private2 {} + +class Public4 implements _Private2 {} + +class _Private1 { + _Private1.fromInt(int i); + _Private1 f = _Private1.fromInt(0); + _Private1 m1(_Private1 p) => _Private1.fromInt(0); +} + +class _Private2 {} + +// Mixins + +mixin Public5 on Public1 implements Public6 {} + +mixin Public6 on _Private1 {} // LINT + +mixin Public7 implements _Private1 {} + +mixin _Private3 on _Private1 implements _Private2 {} + +// Extensions + +extension Public8 on Public1 {} + +extension Public9 on _Private1 {} // LINT + +extension on String {} + +extension _E on int {} + +// Class members + +class PublicClassWithMembers { + // Fields + + String f1 = ''; + _Private1 f2 = _Private1.fromInt(0); // LINT + _Private2 f3 = _Private2(); // LINT + List<_Private1> f4 = []; // LINT + _Private1 _f5 = _Private1.fromInt(0); + + // Constructors + + PublicClassWithMembers(_Private1 p); // LINT + PublicClassWithMembers.c1(_Private1 p); // LINT + PublicClassWithMembers._c2(_Private1 p); + + // Methods + + String m1(int i) => _m4(_Private1.fromInt(i)).toString(); + _Private1 m2(int i) => _Private1.fromInt(0); // LINT + String m3(_Private1 p) => ''; // LINT + _Private1 _m4(_Private1 p) => _Private1.fromInt(0); + + // Operators + + int operator+(_Private1 p) => 0; // LINT + _Private1 operator-(int i) => _Private1.fromInt(i); // LINT + + // Getters + + String get g1 => _g3.toString(); + _Private1 get g2 => _Private1.fromInt(0); // LINT + _Private1 get _g3 => _f5; + + // Setters + + set s1(int i) { + _s3 = _Private1.fromInt(i); + } + set s2(_Private1 i) {} // LINT + set _s3(_Private1 p) {} +} From f8497c807ecb7e337fd7d481826cdd9f2983b7d0 Mon Sep 17 00:00:00 2001 From: Brian Wilkerson Date: Fri, 19 Mar 2021 11:38:16 -0700 Subject: [PATCH 2/3] remove debug code --- lib/src/rules/library_private_types_in_public_api.dart | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/src/rules/library_private_types_in_public_api.dart b/lib/src/rules/library_private_types_in_public_api.dart index 73d62370c..0642fe641 100644 --- a/lib/src/rules/library_private_types_in_public_api.dart +++ b/lib/src/rules/library_private_types_in_public_api.dart @@ -236,11 +236,7 @@ class Visitor extends SimpleAstVisitor { void visitCompilationUnit(CompilationUnit node) { if (!Validator.isPrivate(node.declaredElement!)) { var validator = Validator(rule); - try { - node.declarations.accept(validator); - } catch (e, st) { - DateTime.now(); - } + node.declarations.accept(validator); } } } From 8be4d959eed58fd40edf3184a39d6ac7c960d6ac Mon Sep 17 00:00:00 2001 From: Brian Wilkerson Date: Mon, 22 Mar 2021 14:19:03 -0700 Subject: [PATCH 3/3] address comments --- lib/src/rules.dart | 2 +- .../library_private_types_in_public_api.dart | 22 +++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/src/rules.dart b/lib/src/rules.dart index 83e416fa3..8d3d126e9 100644 --- a/lib/src/rules.dart +++ b/lib/src/rules.dart @@ -270,7 +270,7 @@ void registerLintRules({bool inTestMode = false}) { ..register(LeadingNewlinesInMultilineStrings()) ..register(LibraryNames()) ..register(LibraryPrefixes()) - ..register(LibraryPrivateTypeInPublicAPI()) + ..register(LibraryPrivateTypesInPublicAPI()) ..register(LinesLongerThan80Chars()) ..register(ListRemoveUnrelatedType()) ..register(LiteralOnlyBooleanExpressions()) diff --git a/lib/src/rules/library_private_types_in_public_api.dart b/lib/src/rules/library_private_types_in_public_api.dart index 0642fe641..699831b8b 100644 --- a/lib/src/rules/library_private_types_in_public_api.dart +++ b/lib/src/rules/library_private_types_in_public_api.dart @@ -12,7 +12,20 @@ const _desc = r'Avoid using private types in public APIs.'; const _details = r''' -**AVOID** using private types in public APIs. +**AVOID** using library private types in public APIs. + +For the purposes of this lint, a public API is considered to be any top-level or +member declaration unless the declaration is library private or contained in a +declarartion that's library private. The following uses of types are checked: + +- the return type of a function or method, +- the type of any parameter of a function or method, +- the bound of a type parameter to any function, method, class, mixin, + extension's extended type, or type alias, +- the type of any top level variable or field, +- any type used in the declaration of a type alias (for example + `typedef F = _Private Function();`), or +- any type used in the `on` clause of an extension or a mixin **GOOD:** ``` @@ -27,8 +40,8 @@ class _Private {} '''; -class LibraryPrivateTypeInPublicAPI extends LintRule implements NodeLintRule { - LibraryPrivateTypeInPublicAPI() +class LibraryPrivateTypesInPublicAPI extends LintRule implements NodeLintRule { + LibraryPrivateTypesInPublicAPI() : super( name: 'library_private_types_in_public_api', description: _desc, @@ -234,7 +247,8 @@ class Visitor extends SimpleAstVisitor { @override void visitCompilationUnit(CompilationUnit node) { - if (!Validator.isPrivate(node.declaredElement!)) { + var element = node.declaredElement; + if (element != null && !Validator.isPrivate(element)) { var validator = Validator(rule); node.declarations.accept(validator); }