From 7458627c4bee39557928d191b346448d32af0180 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Fri, 17 Jul 2026 16:24:39 +0300 Subject: [PATCH 1/9] feat: add feature_envy rule --- lib/analysis_options.yaml | 4 + lib/main.dart | 2 + .../lints/feature_envy/feature_envy_rule.dart | 89 ++ .../models/feature_envy_metrics.dart | 67 ++ .../models/feature_envy_parameters.dart | 70 ++ .../models/project_class_cache.dart | 13 + .../utils/member_access_utils.dart | 135 +++ .../visitors/feature_envy_visitor.dart | 57 ++ .../visitors/member_access_visitor.dart | 185 ++++ lib/src/utils/node_utils.dart | 160 ++++ lib/src/utils/types_utils.dart | 57 ++ test/src/common/utils/is_data_class_test.dart | 76 ++ .../feature_envy/feature_envy_rule_test.dart | 798 ++++++++++++++++++ .../models/feature_envy_metrics_test.dart | 78 ++ 14 files changed, 1791 insertions(+) create mode 100644 lib/src/lints/feature_envy/feature_envy_rule.dart create mode 100644 lib/src/lints/feature_envy/models/feature_envy_metrics.dart create mode 100644 lib/src/lints/feature_envy/models/feature_envy_parameters.dart create mode 100644 lib/src/lints/feature_envy/models/project_class_cache.dart create mode 100644 lib/src/lints/feature_envy/utils/member_access_utils.dart create mode 100644 lib/src/lints/feature_envy/visitors/feature_envy_visitor.dart create mode 100644 lib/src/lints/feature_envy/visitors/member_access_visitor.dart create mode 100644 test/src/common/utils/is_data_class_test.dart create mode 100644 test/src/lints/feature_envy/feature_envy_rule_test.dart create mode 100644 test/src/lints/feature_envy/models/feature_envy_metrics_test.dart diff --git a/lib/analysis_options.yaml b/lib/analysis_options.yaml index dd2a9c95..69e63fe1 100644 --- a/lib/analysis_options.yaml +++ b/lib/analysis_options.yaml @@ -63,6 +63,10 @@ solid_lints: max_complexity: 10 double_literal_format: true + feature_envy: + atfd_threshold: 4 + laa_threshold: 0.33 + fdp_threshold: 2 function_lines_of_code: max_lines: 200 diff --git a/lib/main.dart b/lib/main.dart index a6f55fad..d000b39b 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -15,6 +15,7 @@ import 'package:solid_lints/src/lints/avoid_unused_parameters/avoid_unused_param import 'package:solid_lints/src/lints/avoid_using_api/avoid_using_api_rule.dart'; import 'package:solid_lints/src/lints/cyclomatic_complexity/cyclomatic_complexity_rule.dart'; import 'package:solid_lints/src/lints/double_literal_format/double_literal_format_rule.dart'; +import 'package:solid_lints/src/lints/feature_envy/feature_envy_rule.dart'; import 'package:solid_lints/src/lints/function_lines_of_code/function_lines_of_code_rule.dart'; import 'package:solid_lints/src/lints/member_ordering/member_ordering_rule.dart'; import 'package:solid_lints/src/lints/named_parameters_ordering/named_parameters_ordering_rule.dart'; @@ -65,6 +66,7 @@ class SolidLintsPlugin extends Plugin { AvoidUsingApiRule(analysisOptionsLoader: analysisLoader), CyclomaticComplexityRule(analysisOptionsLoader: analysisLoader), DoubleLiteralFormatRule(), + FeatureEnvyRule(analysisOptionsLoader: analysisLoader), FunctionLinesOfCodeRule(analysisOptionsLoader: analysisLoader), MemberOrderingRule(analysisOptionsLoader: analysisLoader), NamedParametersOrderingRule(analysisOptionsLoader: analysisLoader), diff --git a/lib/src/lints/feature_envy/feature_envy_rule.dart b/lib/src/lints/feature_envy/feature_envy_rule.dart new file mode 100644 index 00000000..c2f9e59e --- /dev/null +++ b/lib/src/lints/feature_envy/feature_envy_rule.dart @@ -0,0 +1,89 @@ +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/error/error.dart'; +import 'package:solid_lints/src/lints/feature_envy/models/feature_envy_parameters.dart'; +import 'package:solid_lints/src/lints/feature_envy/visitors/feature_envy_visitor.dart'; +import 'package:solid_lints/src/models/solid_lint_rule.dart'; + +/// Warns if a method accesses fields or methods from a different class +/// more often than from its own class (feature envy). +/// +/// ### Example +/// BAD: +/// ```dart +/// class A { +/// int field; +/// A(this.field); +/// } +/// +/// class B { +/// int method(A a) => a.field * a.field; // LINT +/// } +/// ``` +/// +/// GOOD: +/// ```dart +/// class A { +/// int field; +/// A(this.field); +/// +/// int method() => field * field; +/// } +/// +/// class B { +/// int method(A a) => a.method(); +/// } +/// ``` +/// +/// ### Detection Algorithm +/// The rule detects feature envy using three metrics: +/// - **ATFD** (Access to Foreign Data): Accesses to members of a single +/// external class. Triggers if ATFD >= threshold (default 4). +/// - **LAA** (Locality of Attribute Access): The ratio of internal accesses +/// to total accesses. Triggers if LAA < threshold (default 0.33). +/// - **FDP** (Foreign Data Providers): The number of unique external classes +/// accessed. Triggers if FDP <= threshold (default 2). +/// +/// Accesses to other instances of the same class, non-project classes, +/// closures, nested functions, and data classes are ignored. +class FeatureEnvyRule extends SolidLintRule { + /// Name of the lint. + static const lintName = 'feature_envy'; + + static const _code = LintCode( + lintName, + "Avoid accessing members of `{0}` in `{1}` more often than own members.", + correctionMessage: + "Consider moving the related logic into the `{0}` class.", + ); + + @override + DiagnosticCode get diagnosticCode => _code; + + /// Creates a new instance of [FeatureEnvyRule]. + FeatureEnvyRule({ + required super.analysisOptionsLoader, + }) : super.withParameters( + name: lintName, + description: + 'Warns if a method accesses members of another class more ' + 'often than its own (feature envy).', + parametersParser: FeatureEnvyParameters.fromJson, + ); + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + super.registerNodeProcessors(registry, context); + + final parameters = + getParametersForContext(context) ?? FeatureEnvyParameters.empty(); + + registry.addMethodDeclaration( + this, + FeatureEnvyVisitor(this, parameters), + ); + } +} diff --git a/lib/src/lints/feature_envy/models/feature_envy_metrics.dart b/lib/src/lints/feature_envy/models/feature_envy_metrics.dart new file mode 100644 index 00000000..79d3ccfa --- /dev/null +++ b/lib/src/lints/feature_envy/models/feature_envy_metrics.dart @@ -0,0 +1,67 @@ +import 'package:analyzer/dart/element/element.dart'; +import 'package:collection/collection.dart'; +import 'package:solid_lints/src/lints/feature_envy/models/feature_envy_parameters.dart'; + +/// Calculated Feature Envy metrics for a method. +class FeatureEnvyMetrics { + /// Locality of Attribute Access (LAA) metric. + final double laa; + + /// Foreign Data Providers (FDP) metric. + final int fdp; + + /// Access to Foreign Data (ATFD) metric. + final int atfd; + + /// The class element that is accessed the most externally. + final InterfaceElement? maxEnvyElement; + + const FeatureEnvyMetrics._({ + required this.laa, + required this.fdp, + required this.atfd, + required this.maxEnvyElement, + }); + + /// Checks if these metrics exceed the thresholds defined in [parameters], + /// indicating a feature envy code smell. + bool exceedsThresholds(FeatureEnvyParameters parameters) => + atfd >= parameters.atfdThreshold && + laa < parameters.laaThreshold && + fdp <= parameters.fdpThreshold; + + /// Calculates metrics based on collected accesses. + factory FeatureEnvyMetrics.calculate({ + required int internalAccesses, + required Map externalAccessCounts, + }) { + final totalExternalAccesses = externalAccessCounts.values.sum; + final totalAccesses = internalAccesses + totalExternalAccesses; + + final laa = totalAccesses == 0 ? 1.0 : internalAccesses / totalAccesses; + final fdp = externalAccessCounts.length; + + int maxExternalCount = 0; + InterfaceElement? maxElement; + + for (final MapEntry(:key, :value) in externalAccessCounts.entries) { + final isLarger = value > maxExternalCount; + final isTieWithSmallerName = + value == maxExternalCount && + maxElement != null && + (key.name ?? '').compareTo(maxElement.name ?? '') < 0; + + if (isLarger || isTieWithSmallerName) { + maxExternalCount = value; + maxElement = key; + } + } + + return FeatureEnvyMetrics._( + laa: laa, + fdp: fdp, + atfd: maxExternalCount, + maxEnvyElement: maxElement, + ); + } +} diff --git a/lib/src/lints/feature_envy/models/feature_envy_parameters.dart b/lib/src/lints/feature_envy/models/feature_envy_parameters.dart new file mode 100644 index 00000000..49895aa8 --- /dev/null +++ b/lib/src/lints/feature_envy/models/feature_envy_parameters.dart @@ -0,0 +1,70 @@ +import 'package:solid_lints/src/common/parameters/excluded_identifiers_list_parameter.dart'; + +/// Configuration parameters for the feature_envy rule. +class FeatureEnvyParameters { + /// A list of methods that should be excluded from the lint. + final ExcludedIdentifiersListParameter exclude; + + /// Access to Foreign Data (ATFD) threshold. + /// Triggered if ATFD >= atfdThreshold. + final int atfdThreshold; + + /// Locality of Attribute Access (LAA) threshold. + /// Triggered if LAA < laaThreshold. + final double laaThreshold; + + /// Foreign Data Providers (FDP) threshold. + /// Triggered only if FDP <= fdpThreshold. + final int fdpThreshold; + + /// Default Access to Foreign Data (ATFD) threshold. + static const _defaultAtfdThreshold = 4; + + /// Default Locality of Attribute Access (LAA) threshold. + static const _defaultLaaThreshold = 0.33; + + /// Default Foreign Data Providers (FDP) threshold. + static const _defaultFdpThreshold = 2; + + /// Constructor for [FeatureEnvyParameters] model. + const FeatureEnvyParameters({ + required this.atfdThreshold, + required this.exclude, + required this.laaThreshold, + required this.fdpThreshold, + }); + + /// Empty [FeatureEnvyParameters] model with default values. + factory FeatureEnvyParameters.empty() => FeatureEnvyParameters( + atfdThreshold: _defaultAtfdThreshold, + exclude: ExcludedIdentifiersListParameter(exclude: []), + laaThreshold: _defaultLaaThreshold, + fdpThreshold: _defaultFdpThreshold, + ); + + /// Creates a [FeatureEnvyParameters] model from JSON data. + factory FeatureEnvyParameters.fromJson(Map json) { + final atfdThreshold = switch (json['atfd_threshold']) { + final int value => value, + _ => _defaultAtfdThreshold, + }; + + final laaThreshold = switch (json['laa_threshold']) { + final double value => value, + final int value => value.toDouble(), + _ => _defaultLaaThreshold, + }; + + final fdpThreshold = switch (json['fdp_threshold']) { + final int value => value, + _ => _defaultFdpThreshold, + }; + + return FeatureEnvyParameters( + exclude: ExcludedIdentifiersListParameter.defaultFromJson(json), + atfdThreshold: atfdThreshold, + laaThreshold: laaThreshold, + fdpThreshold: fdpThreshold, + ); + } +} diff --git a/lib/src/lints/feature_envy/models/project_class_cache.dart b/lib/src/lints/feature_envy/models/project_class_cache.dart new file mode 100644 index 00000000..3d5a4683 --- /dev/null +++ b/lib/src/lints/feature_envy/models/project_class_cache.dart @@ -0,0 +1,13 @@ +import 'package:analyzer/dart/element/element.dart'; +import 'package:solid_lints/src/utils/node_utils.dart'; + +/// A cache for checking if an [InterfaceElement] belongs to the analyzed +/// project. +class ProjectClassCache { + final _cache = {}; + + /// Returns `true` if [element] belongs to the analyzed project. + /// Results are cached for better performance during static analysis. + bool isProjectClass(InterfaceElement element) => + _cache.putIfAbsent(element, () => element.isFromProject); +} diff --git a/lib/src/lints/feature_envy/utils/member_access_utils.dart b/lib/src/lints/feature_envy/utils/member_access_utils.dart new file mode 100644 index 00000000..08a6cfa0 --- /dev/null +++ b/lib/src/lints/feature_envy/utils/member_access_utils.dart @@ -0,0 +1,135 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:solid_lints/src/lints/feature_envy/models/project_class_cache.dart'; +import 'package:solid_lints/src/utils/node_utils.dart'; +import 'package:solid_lints/src/utils/types_utils.dart'; + +/// Utility methods for analyzing member accesses within the feature envy lint. +abstract final class MemberAccessUtils { + /// Resolves the base element of a member access target expression. + static Element? resolveTargetElement( + Expression? target, { + required bool isPatternField, + }) { + final unwrapped = target?.unwrapTarget; + final baseElement = switch (unwrapped) { + _ when isPatternField => null, + ExtensionOverride( + argumentList: ArgumentList(arguments: [final firstArg, ...]), + ) => + firstArg.staticType?.element, + final expr? => expr.staticType?.element, + _ => null, + }; + + var resolved = baseElement?.resolveTypeParameter; + if (resolved case ExtensionElement(:final extendedType)) { + resolved = extendedType.element?.resolveTypeParameter; + } + + return resolved; + } + + /// Checks if the access to [c] is considered internal to [currentClass]. + static bool isInternalAccess( + Expression? target, + InterfaceElement c, { + required InterfaceElement currentClass, + bool isPatternField = false, + }) { + if (!currentClass.isSameOrSubclassOf(c)) return false; + if (target == null && isPatternField) return false; + + final unwrapped = target?.unwrapTarget; + + return switch (unwrapped) { + ExtensionOverride( + argumentList: ArgumentList(arguments: [final firstArg, ...]), + ) => + firstArg.unwrapTarget.isThisOrSuper, + _ => unwrapped.isThisOrSuperOrNull, + }; + } + + /// Finds the highest level access expression that starts with [node]. + static Expression getAccessExpression(SimpleIdentifier node) => + findAccessExpression(node, node.parent); + + /// Recursively finds the top-most expression of an access. + static Expression findAccessExpression(Expression child, AstNode? parent) => + switch (parent) { + ParenthesizedExpression(:final expression) when expression == child => + findAccessExpression(parent, parent.parent), + PropertyAccess(:final propertyName, :final target) + when propertyName == child && target.unwrapTarget.isThisOrSuper => + findAccessExpression(parent, parent.parent), + _ => child, + }; + + /// Determines if [node] is the target of an access to an external member. + static bool isTargetOfExternalAccess( + SimpleIdentifier node, { + required InterfaceElement currentClass, + required ProjectClassCache projectClassCache, + }) { + if (node.parent == null) return false; + + final element = node.element; + if (element == null) return false; + + final enclosing = element.enclosingInterface; + if (enclosing == null || !currentClass.isSameOrSubclassOf(enclosing)) { + return false; + } + + final expr = getAccessExpression(node); + final exprParent = expr.parent; + if (exprParent == null) return false; + + return switch (exprParent) { + MethodInvocation(:final target, :final methodName) when target == expr => + isExternalMember( + methodName.element, + currentClass: currentClass, + projectClassCache: projectClassCache, + ), + PrefixedIdentifier(:final prefix, :final identifier) + when prefix == expr => + isExternalMember( + identifier.element, + currentClass: currentClass, + projectClassCache: projectClassCache, + ), + PropertyAccess(:final target, :final propertyName) when target == expr => + isExternalMember( + propertyName.element, + currentClass: currentClass, + projectClassCache: projectClassCache, + ), + CascadeExpression(:final target, :final cascadeSections) + when target == expr => + cascadeSections.any( + (s) => isExternalMember( + s.memberElement, + currentClass: currentClass, + projectClassCache: projectClassCache, + ), + ), + _ => false, + }; + } + + /// Determines if [element] represents a member of an external class. + static bool isExternalMember( + Element? element, { + required InterfaceElement currentClass, + required ProjectClassCache projectClassCache, + }) { + final enclosing = element?.enclosingInterface; + if (enclosing == null) return false; + + return !currentClass.isSameOrSubclassOf(enclosing) && + projectClassCache.isProjectClass(enclosing) && + !enclosing.isDataClass; + } +} diff --git a/lib/src/lints/feature_envy/visitors/feature_envy_visitor.dart b/lib/src/lints/feature_envy/visitors/feature_envy_visitor.dart new file mode 100644 index 00000000..27fc53eb --- /dev/null +++ b/lib/src/lints/feature_envy/visitors/feature_envy_visitor.dart @@ -0,0 +1,57 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:solid_lints/src/lints/feature_envy/feature_envy_rule.dart'; +import 'package:solid_lints/src/lints/feature_envy/models/feature_envy_metrics.dart'; +import 'package:solid_lints/src/lints/feature_envy/models/feature_envy_parameters.dart'; +import 'package:solid_lints/src/lints/feature_envy/models/project_class_cache.dart'; +import 'package:solid_lints/src/lints/feature_envy/visitors/member_access_visitor.dart'; +import 'package:solid_lints/src/utils/types_utils.dart'; + +/// The AST visitor that checks every method for feature envy. +class FeatureEnvyVisitor extends SimpleAstVisitor { + final FeatureEnvyRule _rule; + final FeatureEnvyParameters _parameters; + final _projectClassCache = ProjectClassCache(); + + /// Creates a new instance of [FeatureEnvyVisitor]. + FeatureEnvyVisitor(this._rule, this._parameters); + + @override + void visitMethodDeclaration(MethodDeclaration node) { + if (node.isStatic) return; // ignore static methods + if (node.body case EmptyFunctionBody()) return; + if (_parameters.exclude.shouldIgnore(node)) return; + + final interfaceElement = node.declaredFragment?.element.enclosingElement; + if (interfaceElement is! InterfaceElement) return; + + final classType = interfaceElement.thisType; + if (isWidgetOrSubclass(classType) || isWidgetStateOrSubclass(classType)) { + return; + } + + final accessVisitor = MemberAccessVisitor( + interfaceElement, + _projectClassCache, + ); + node.accept(accessVisitor); + + if (accessVisitor.externalAccessCounts.isEmpty) return; + + final metrics = FeatureEnvyMetrics.calculate( + internalAccesses: accessVisitor.internalAccesses, + externalAccessCounts: accessVisitor.externalAccessCounts, + ); + + if (metrics.exceedsThresholds(_parameters)) { + _rule.reportAtToken( + node.name, + arguments: [ + metrics.maxEnvyElement?.name ?? '', + interfaceElement.name ?? '', + ], + ); + } + } +} diff --git a/lib/src/lints/feature_envy/visitors/member_access_visitor.dart b/lib/src/lints/feature_envy/visitors/member_access_visitor.dart new file mode 100644 index 00000000..e110802a --- /dev/null +++ b/lib/src/lints/feature_envy/visitors/member_access_visitor.dart @@ -0,0 +1,185 @@ +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:solid_lints/src/lints/feature_envy/models/project_class_cache.dart'; +import 'package:solid_lints/src/lints/feature_envy/utils/member_access_utils.dart'; +import 'package:solid_lints/src/utils/node_utils.dart'; +import 'package:solid_lints/src/utils/types_utils.dart'; + +/// A visitor that collects internal and external member accesses for a class. +class MemberAccessVisitor extends RecursiveAstVisitor { + final InterfaceElement _currentClass; + final ProjectClassCache _projectClassCache; + + /// The number of accesses to members of the current class. + int internalAccesses = 0; + + /// The counts of accesses to members of external classes. + final externalAccessCounts = {}; + + /// Creates a new instance of [MemberAccessVisitor]. + MemberAccessVisitor(this._currentClass, this._projectClassCache); + + @override + void visitSimpleIdentifier(SimpleIdentifier node) { + super.visitSimpleIdentifier(node); + + final element = node.element; + if (element == null || element is SetterElement) return; + + if (MemberAccessUtils.isTargetOfExternalAccess( + node, + currentClass: _currentClass, + projectClassCache: _projectClassCache, + )) { + return; + } + + if (element.isInstanceMember) { + _processElement( + element: element, + target: node.memberAccessTarget, + ); + } + } + + @override + void visitPatternField(PatternField node) { + super.visitPatternField(node); + + if (node.element case final element? when element.isInstanceMember) { + _processElement( + element: element, + target: node.parent?.matchedPatternExpression, + isPatternField: true, + ); + } + } + + @override + void visitBinaryExpression(BinaryExpression node) { + super.visitBinaryExpression(node); + + if (node.element case final element? + when element.isInstanceMethod && !node.isNullCheck) { + _processElement( + element: element, + target: node.leftOperand, + ); + } + } + + @override + void visitIndexExpression(IndexExpression node) { + super.visitIndexExpression(node); + + if (node.element case final element? when element.isInstanceMethod) { + _processElement( + element: element, + target: node.realTarget, + ); + } + } + + void _processOperatorAndWrite( + Element? opElement, + Element? writeElement, + Expression operand, + ) { + if (opElement case final e? when e.isInstanceMethod) { + _processElement(element: e, target: operand); + } + if (writeElement case final e? when e.isInstanceMember) { + _processElement(element: e, target: operand.targetExpression); + } + } + + @override + void visitPrefixExpression(PrefixExpression node) { + super.visitPrefixExpression(node); + _processOperatorAndWrite(node.element, node.writeElement, node.operand); + } + + @override + void visitPostfixExpression(PostfixExpression node) { + super.visitPostfixExpression(node); + _processOperatorAndWrite(node.element, node.writeElement, node.operand); + } + + @override + void visitAssignmentExpression(AssignmentExpression node) { + super.visitAssignmentExpression(node); + + _processOperatorAndWrite( + node.element, + node.writeElement, + node.leftHandSide, + ); + } + + @override + void visitFunctionExpressionInvocation(FunctionExpressionInvocation node) { + super.visitFunctionExpressionInvocation(node); + + if (node.function.staticType case InterfaceType(:final element)) { + if (element.getMethod('call') case final callMethod? + when callMethod.isInstanceMethod) { + _processElement( + element: callMethod, + target: node.function, + ); + } + } + } + + @override + void visitFunctionExpression(FunctionExpression node) { + // Stop traversal to ignore accesses inside closures. + } + + @override + void visitFunctionDeclaration(FunctionDeclaration node) { + // Stop traversal to ignore accesses inside nested functions. + } + + void _processElement({ + required Element element, + required Expression? target, + bool isPatternField = false, + }) { + if (target == null && element.enclosingElement is ExtensionElement) { + internalAccesses++; + return; + } + + final targetElement = MemberAccessUtils.resolveTargetElement( + target, + isPatternField: isPatternField, + ); + + final enclosingInterface = (targetElement ?? element).enclosingInterface; + if (enclosingInterface == null) return; + + if (MemberAccessUtils.isInternalAccess( + target, + enclosingInterface, + currentClass: _currentClass, + isPatternField: isPatternField, + )) { + internalAccesses++; + return; + } + + if (enclosingInterface == _currentClass) return; + + if (_projectClassCache.isProjectClass(enclosingInterface) && + !enclosingInterface.isDataClass) { + externalAccessCounts.update( + enclosingInterface, + (count) => count + 1, + ifAbsent: () => 1, + ); + } + } +} diff --git a/lib/src/utils/node_utils.dart b/lib/src/utils/node_utils.dart index 9b526184..f509b868 100644 --- a/lib/src/utils/node_utils.dart +++ b/lib/src/utils/node_utils.dart @@ -63,6 +63,22 @@ extension SimpleIdentifierExtension on SimpleIdentifier { /// Returns the library URI string of the element, or null. String? get sourceUrl => element?.libraryUri; + + /// Returns the target expression if this identifier is part of a member + /// access (e.g., `target.member`, `target.method()`, `target.field`), + /// otherwise null. + Expression? get memberAccessTarget => switch (parent) { + PropertyAccess(:final propertyName, :final realTarget) + when propertyName == this => + realTarget, + MethodInvocation(:final methodName, :final realTarget) + when methodName == this => + realTarget, + PrefixedIdentifier(:final identifier, :final prefix) + when identifier == this => + prefix, + _ => null, + }; } /// Extension on [AstNode] to provide generic context/traversal checks. @@ -107,6 +123,32 @@ extension AstNodeExtension on AstNode { final p = parent is PrefixExpression ? parent?.parent : parent; return p is IndexExpression; } + + /// Traverses up the AST from a pattern node to find the root expression + /// being matched. + Expression? get matchedPatternExpression { + AstNode? current = this; + while (current != null) { + switch (current) { + case PatternField() || + ListPattern() || + MapPattern() || + RecordPattern() || + ForEachPartsWithPattern(): + return null; + case PatternVariableDeclaration(:final expression) || + PatternAssignment(:final expression) || + SwitchStatement(:final expression) || + SwitchExpression(:final expression) || + IfStatement(:final expression) || + IfElement(:final expression): + return expression; + default: + current = current.parent; + } + } + return null; + } } /// Extension on [NamedType] to provide source URL utility. @@ -145,6 +187,53 @@ extension ElementExtension on Element { return false; } + + /// Checks whether this element is a non-static instance member. + bool get isInstanceMember => switch (this) { + PropertyAccessorElement(:final isStatic) => !isStatic, + MethodElement(:final isStatic) => !isStatic, + FieldElement(:final isStatic) => !isStatic, + _ => false, + }; + + /// Checks whether this element is a non-static instance method. + bool get isInstanceMethod => this is MethodElement && isInstanceMember; + + /// Checks whether this element belongs to the analyzed project. + bool get isFromProject { + final session = this.session; + if (session == null) return false; + + final sourcePath = library?.firstFragment.source.fullName; + if (sourcePath == null) return false; + + return session.analysisContext.contextRoot.isAnalyzed(sourcePath); + } + + /// Returns the enclosing [InterfaceElement] of this element (recursively), + /// or null if none. + InterfaceElement? get enclosingInterface { + Element? current = this; + while (current != null) { + if (current case InterfaceElement()) { + return current; + } + current = current.enclosingElement; + } + return null; + } + + /// Resolves type parameters to their bounds recursively to prevent cycle + /// loops. + Element? get resolveTypeParameter { + Element? current = this; + final visited = {}; + while (current is TypeParameterElement) { + if (!visited.add(current)) break; + current = current.bound?.element; + } + return current; + } } /// Extension on [VariableDeclaration] to check declared type. @@ -155,3 +244,74 @@ extension VariableDeclarationExtension on VariableDeclaration { return element is VariableElement ? element.type : null; } } + +/// Extension on [BinaryExpression] to check binary expressions. +extension BinaryExpressionExtension on BinaryExpression { + /// Returns `true` if this is an equality expression (== or !=). + bool get isEquality => + operator.type == TokenType.EQ_EQ || operator.type == TokenType.BANG_EQ; + + /// Returns `true` if one of the operands is a null literal. + bool get hasNullOperand => + leftOperand is NullLiteral || rightOperand is NullLiteral; + + /// Returns `true` if this expression is a null check. + bool get isNullCheck => isEquality && hasNullOperand; +} + +/// Extension on [Expression] to provide target unwrapping and resolving +/// utilities. +extension ExpressionExtension on Expression { + /// Returns the target of a property access, prefixed identifier, or index + /// expression, or null for other expressions. + Expression? get targetExpression => switch (this) { + PropertyAccess(:final realTarget) => realTarget, + PrefixedIdentifier(:final prefix) => prefix, + IndexExpression(:final realTarget) => realTarget, + _ => null, + }; + + /// Returns the member element referenced or operated on by this expression, + /// or null if none. + Element? get memberElement => switch (this) { + MethodInvocation(:final methodName) => methodName.element, + PropertyAccess(:final propertyName) => propertyName.element, + AssignmentExpression(:final writeElement, :final readElement) => + writeElement ?? readElement, + PrefixExpression(:final writeElement, :final readElement) => + writeElement ?? readElement, + PostfixExpression(:final writeElement, :final readElement) => + writeElement ?? readElement, + IndexExpression(:final element) => element, + BinaryExpression(:final element) => element, + _ => null, + }; +} + +/// Extension on nullable [Expression] to provide helper checks. +extension ExpressionNullableExtension on Expression? { + /// Unwraps casts ([AsExpression]) and null assertions (e.g. `expr!`) + /// recursively down to the core expression. + Expression? get unwrapTarget { + var current = this?.unParenthesized; + while (true) { + if (current case AsExpression(:final expression)) { + current = expression.unParenthesized; + } else if (current case PostfixExpression( + :final operand, + :final operator, + ) when operator.type == TokenType.BANG) { + current = operand.unParenthesized; + } else { + return current; + } + } + } + + /// Returns `true` if this expression is `this` or `super` or null. + bool get isThisOrSuperOrNull => + this == null || this is ThisExpression || this is SuperExpression; + + /// Returns `true` if this expression is `this` or `super`. + bool get isThisOrSuper => this is ThisExpression || this is SuperExpression; +} diff --git a/lib/src/utils/types_utils.dart b/lib/src/utils/types_utils.dart index 64558270..33878070 100644 --- a/lib/src/utils/types_utils.dart +++ b/lib/src/utils/types_utils.dart @@ -83,6 +83,63 @@ extension TypeString on String { String replaceGenericString() => replaceFirst(_genericRegex, ''); } +extension _PropertyAccessorElementExt on PropertyAccessorElement { + bool get isPublicInstanceOrigin => + isPublic && !isStatic && isOriginDeclaration; +} + +extension InterfaceElementExt on InterfaceElement { + /// Checks whether this element is the same as or a subclass of [superType]. + bool isSameOrSubclassOf(InterfaceElement superType) => + this == superType || + allSupertypes.any((type) => type.element == superType); + + /// Checks if a class is a Data Class based on Weight of Class. + bool get isDataClass { + const dataClassWeightThreshold = 0.33; + + final elements = [ + this, + ...allSupertypes.where((s) => !s.isDartCoreObject).map((s) => s.element), + ]; + + final publicProperties = { + for (final el in elements) ...[ + ...el.getters + .where((g) => g.isPublicInstanceOrigin) + .map((g) => g.displayName) + .where((name) => !const {'hashCode', 'runtimeType'}.contains(name)), + ...el.setters + .where((s) => s.isPublicInstanceOrigin) + .map((s) => s.displayName), + ], + }; + + final publicMethods = { + for (final el in elements) + ...el.methods + .where((m) => m.isPublic && !m.isStatic) + .map((m) => m.name) + .nonNulls + .where( + (name) => !const { + 'toString', + '==', + 'copyWith', + 'toJson', + 'noSuchMethod', + }.contains(name), + ), + }; + + final totalMembers = publicProperties.length + publicMethods.length; + if (totalMembers == 0) return true; + + final weightOfClass = publicMethods.length / totalMembers; + return weightOfClass < dataClassWeightThreshold; + } +} + bool hasWidgetType(DartType type) => (isWidgetOrSubclass(type) || _isIterable(type) || diff --git a/test/src/common/utils/is_data_class_test.dart b/test/src/common/utils/is_data_class_test.dart new file mode 100644 index 00000000..9ff01f5a --- /dev/null +++ b/test/src/common/utils/is_data_class_test.dart @@ -0,0 +1,76 @@ +import 'package:analyzer/dart/analysis/results.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer_testing/src/analysis_rule/pub_package_resolution.dart'; +import 'package:solid_lints/src/utils/types_utils.dart'; +import 'package:test/test.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(IsDataClassTest); + }); +} + +@reflectiveTest +class IsDataClassTest extends PubPackageResolutionTest { + Future _resolveCode(String code) async { + newFile(testFile.path, code); + return resolveFile(testFile.path); + } + + Future test_isDataClass_for_pure_data_class() async { + final resolved = await _resolveCode(''' +class Rectangle { + final int width; + final int height; + const Rectangle(this.width, this.height); +} +'''); + final classDecl = resolved.unit.declarations + .whereType() + .first; + final element = classDecl.declaredFragment?.element; + expect(element, isNotNull); + + expect(element?.isDataClass, isTrue); + } + + Future test_isDataClass_for_non_data_class() async { + final resolved = await _resolveCode(''' +class Rectangle { + final int width; + final int height; + const Rectangle(this.width, this.height); + int area() => width * height; + void printMe() {} +} +'''); + final classDecl = resolved.unit.declarations + .whereType() + .first; + final element = classDecl.declaredFragment?.element; + expect(element, isNotNull); + + expect(element?.isDataClass, isFalse); + } + + Future test_isDataClass_collision_case() async { + final resolved = await _resolveCode(''' +class Base { + int get foo => 42; +} + +class Sub extends Base { + @override + int foo() => 42; +} +'''); + final subDecl = resolved.unit.declarations + .whereType() + .firstWhere((d) => d.namePart.typeName.lexeme == 'Sub'); + final element = subDecl.declaredFragment?.element; + expect(element, isNotNull); + + expect(element?.isDataClass, isFalse); + } +} diff --git a/test/src/lints/feature_envy/feature_envy_rule_test.dart b/test/src/lints/feature_envy/feature_envy_rule_test.dart new file mode 100644 index 00000000..bf413b10 --- /dev/null +++ b/test/src/lints/feature_envy/feature_envy_rule_test.dart @@ -0,0 +1,798 @@ +import 'package:analyzer_testing/analysis_rule/analysis_rule.dart'; +import 'package:analyzer_testing/utilities/utilities.dart'; +import 'package:solid_lints/src/common/parameter_parser/analysis_options_loader.dart'; +import 'package:solid_lints/src/lints/feature_envy/feature_envy_rule.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import '../../../lints/auto_test_lint_offsets.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(FeatureEnvyRuleTest); + }); +} + +@reflectiveTest +class FeatureEnvyRuleTest extends AnalysisRuleTest with AutoTestLintOffsets { + static const _importFlutterWidgets = "import 'package:flutter/widgets.dart';"; + static const _mockFlutterWidgetsContent = ''' +abstract class Widget { + const Widget(); +} + +class StatelessWidget implements Widget { + const StatelessWidget(); +} + +class StatefulWidget implements Widget { + const StatefulWidget(); +} + +abstract class State { + T get widget => throw 'unimplemented'; +} +'''; + + static const _mockAnalysisOptionsContent = ''' +plugins: + solid_lints: + diagnostics: + feature_envy: + atfd_threshold: 2 + fdp_threshold: 5 + '''; + + @override + void setUp() { + rule = FeatureEnvyRule( + analysisOptionsLoader: AnalysisOptionsLoader( + resourceProvider: resourceProvider, + ), + ); + newPackage('flutter') + ..addFile('lib/widgets.dart', _mockFlutterWidgetsContent); + super.setUp(); + + newAnalysisOptionsYamlFile( + testPackageRootPath, + '''${analysisOptionsContent(rules: [rule.name])} +$_mockAnalysisOptionsContent''', + ); + } + + Future test_reports_when_atfd_exceeds_threshold() => + assertAutoDiagnostics(''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + + int get area => width * height; + int getArea() => width * height; + void dummy() {} +} + +class MathHelper { + int ${expectLint('sumOfAreas')}(Rectangle r1, Rectangle r2) => (r1.width * r1.height) + (r2.width * r2.height); +} +'''); + + Future test_does_not_report_when_atfd_below_threshold() => + assertNoDiagnostics(r''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + int get area => width * height; +} + +class MathHelper { + int getWidth(Rectangle r1) => r1.width; +} +'''); + + Future test_reports_when_atfd_meets_threshold() => + assertAutoDiagnostics(''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + int get area => width * height; + int getArea() => width * height; + void dummy() {} +} + +class MathHelper { + int ${expectLint('sumOfAreas')}(Rectangle r1, Rectangle r2) => r1.area + r2.area; +} +'''); + + Future test_reports_when_external_exceeds_internal() => + assertAutoDiagnostics(''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + int get area => width * height; + void dummy() {} + void dummy2() {} +} + +class NormalClass { + int value = 0; + + void ${expectLint('doSomething')}(Rectangle rect) { + print(rect.width); + print(rect.height); + print(rect.area); + } +} +'''); + + Future test_does_not_report_when_internal_exceeds_external() => + assertNoDiagnostics(r''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + int get area => width * height; +} + +class NormalClass { + int value = 0; + + void doSomethingElse(Rectangle rect) { + print(rect.width); + print(rect.height); + print(value); + print(value); + print(value); + } +} +'''); + + Future test_reports_class_with_most_accesses() => + assertAutoDiagnostics(''' +class Circle { + final int radius; + const Circle(this.radius); + int get area => radius * radius; + void dummy() {} + void dummy2() {} +} + +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + int get area => width * height; + void dummy() {} + void dummy2() {} +} + +class MathHelper { + int ${expectLint('sumOfAreas', name: 'feature_envy', messageContainsAll: ['Rectangle'])}(Rectangle r1, Rectangle r2, Circle c1) { + return (r1.width * r1.height) + (r2.width * r2.height) + c1.area; + } +} +'''); + + Future test_reports_when_accessing_inherited_methods() => + assertAutoDiagnostics(''' +class BaseExternal { + int get baseValue => 1; + void dummy() {} +} + +class External extends BaseExternal { + int get childValue => 2; +} + +class MyClass { + int ${expectLint('doSomething')}(External ext) { + return ext.baseValue + ext.childValue; + } +} +'''); + + Future test_reports_when_using_fields_as_targets() => + assertAutoDiagnostics(''' +class External { + void step1() {} + void step2() {} + void step3() {} +} + +class MyClass { + final External myField = External(); + + void ${expectLint('doSomething')}() { + myField.step1(); + myField.step2(); + myField.step3(); + } +} +'''); + + Future test_reports_when_using_fields_with_this_as_targets() => + assertAutoDiagnostics(''' +class External { + void step1() {} + void step2() {} + void step3() {} +} + +class MyClass { + final External myField = External(); + + void ${expectLint('doSomething')}() { + this.myField.step1(); + this.myField.step2(); + this.myField.step3(); + } +} +'''); + + Future test_reports_with_cascades() => assertAutoDiagnostics(''' +class External { + void step1() {} + void step2() {} + void step3() {} +} + +class MyClass { + final External myField = External(); + + void ${expectLint('doSomething')}() { + myField..step1()..step2()..step3(); + } +} +'''); + + Future test_reports_with_cascaded_assignments() => + assertAutoDiagnostics(''' +class External { + int value1 = 0; + int value2 = 0; + int value3 = 0; + void dummy1() {} + void dummy2() {} + void dummy3() {} +} + +class MyClass { + final External myField = External(); + + void ${expectLint('doSomething')}() { + myField..value1 = 1..value2 = 2..value3 = 3; + } +} +'''); + + Future test_reports_when_accessing_nested_foreign_fields() => + assertAutoDiagnostics(''' +class DeepService { + void execute() {} +} + +class Middleware { + final DeepService service = DeepService(); +} + +class Client { + final Middleware middleware = Middleware(); +} + +class MyClass { + void ${expectLint('process')}(Client client) { + client.middleware.service.execute(); + client.middleware.service.execute(); + client.middleware.service.execute(); + } +} +'''); + + Future test_reports_with_operator_overloads() => + assertAutoDiagnostics(''' +class Vector { + int x; + int y; + Vector(this.x, this.y); + + Vector operator +(Vector other) => Vector(x + other.x, y + other.y); +} + +class MathProcessor { + void ${expectLint('process')}(Vector v1, Vector v2, Vector v3) { + final result = v1 + v2 + v3; + print(result.x); + } +} +'''); + + Future test_patterns_threshold_2() => assertAutoDiagnostics(''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + void dummy() {} +} + +class MathHelper { + void ${expectLint('doSomething')}(Rectangle r) { + if (r case Rectangle(:final height, :final width)) { + print(height); + } + } +} +'''); + + Future test_patterns_no_diagnostic_below_threshold() => + assertNoDiagnostics(''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); +} + +class MathHelper { + void doSomething(Rectangle r) { + if (r case Rectangle(:final height)) { + print(height); + } + } +} +'''); + + Future test_patterns_explicit_no_diagnostic_below_threshold() => + assertNoDiagnostics(''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); +} + +class MathHelper { + void doSomething(Rectangle r) { + if (r case Rectangle(height: final h)) { + print(h); + } + } +} +'''); + + Future test_index_assignment_should_trigger_lint() => + assertAutoDiagnostics(''' +class MyList { + int operator [](int index) => 0; + void operator []=(int index, int value) {} +} + +class MathHelper { + void ${expectLint('doSomething')}(MyList list) { + list[0] = 5; + list[1] = 6; + } +} +'''); + + Future test_compound_index_assignment_should_trigger_lint() => + assertAutoDiagnostics(''' +class MyList { + int operator [](int index) => 0; + void operator []=(int index, int value) {} +} + +class MathHelper { + void ${expectLint('doSomething')}(MyList list) { + list[0] += 5; + list[0] += 6; + } +} +'''); + + Future test_static_member_accesses_ignored() => assertNoDiagnostics(''' +class External { + static int value = 0; + static void staticMethod() {} +} + +class MathHelper { + void doSomething() { + External.value = 5; + External.staticMethod(); + External.staticMethod(); + } +} +'''); + + Future test_constructor_invocations_ignored() => assertNoDiagnostics(''' +class External { + const External(); +} + +class MathHelper { + void doSomething() { + final e1 = External(); + final e2 = External(); + final e3 = const External(); + } +} +'''); + + Future test_extension_override_on_this_internal() => + assertNoDiagnostics(''' +extension MyExtension on MathHelper { + void extMethod() {} +} + +class MathHelper { + void doSomething() { + MyExtension(this).extMethod(); + MyExtension(this).extMethod(); + } +} +'''); + + Future test_explicit_extension_override_on_external_target() => + assertAutoDiagnostics(''' +extension MyExtension on Rectangle { + void extMethod() {} +} + +class Rectangle { + final int width; + final int height; + const Rectangle(this.width, this.height); + void dummy() {} + void dummy2() {} +} + +class MathHelper { + void ${expectLint('doSomething')}(Rectangle r) { + MyExtension(r).extMethod(); + MyExtension(r).extMethod(); + } +} +'''); + + Future test_default_threshold_is_four() async { + newAnalysisOptionsYamlFile( + testPackageRootPath, + analysisOptionsContent(rules: [rule.name]), + ); + + await assertNoDiagnostics(r''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + int get area => width * height; +} + +class MathHelper { + void doSomething(Rectangle r) { + print(r.width); + print(r.height); + print(r.area); + } +} +'''); + + await assertAutoDiagnostics(r''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + int get area => width * height; + int getArea() => width * height; +} + +class MathHelper { + void ${expectLint('doSomething')}(Rectangle r) { + print(r.width); + print(r.height); + print(r.area); + print(r.getArea()); + } +} +'''); + } + + Future test_does_not_count_accesses_in_closures() => + assertNoDiagnostics(r''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); +} + +class MathHelper { + void doSomething(List list) { + list.forEach((r) { + print(r.width); + print(r.height); + print(r.width); + }); + } +} +'''); + + Future test_does_not_count_accesses_in_nested_functions() => + assertNoDiagnostics(r''' +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); +} + +class MathHelper { + void doSomething(Rectangle r) { + void helper() { + print(r.width); + print(r.height); + print(r.width); + } + helper(); + } +} +'''); + + Future test_does_not_report_on_widgets_and_states() => + assertNoDiagnostics(''' +$_importFlutterWidgets + +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); +} + +class MyWidget extends StatelessWidget { + Widget build(Rectangle r) { + print(r.width); + print(r.height); + print(r.width); + return MyWidget(); + } +} + +class MyWidgetState extends State { + void doSomething(Rectangle r) { + print(r.width); + print(r.height); + print(r.width); + } +} +'''); + + Future test_reports_on_cascades_of_instance_creation() => + assertAutoDiagnostics(''' +class External { + void step1() {} + void step2() {} + void step3() {} +} + +class MyClass { + void ${expectLint('doSomething')}() { + External()..step1()..step2()..step3(); + } +} +'''); + + Future test_profile_notifier_mimic() => assertNoDiagnostics(r''' +class Model { + final String field1; + final String field2; + + const Model({ + required this.field1, + required this.field2, + }); +} + +class Service { + Model? get model => null; +} + +class PageNotifier { + String _val1 = ''; + String _val2 = ''; + + void updateFromModel(Service service) { + final data = service.model; + if (data == null) return; + + _val1 = data.field1; + _val2 = data.field2; + } +} +'''); + + Future test_custom_laa_and_fdp_thresholds() async { + newAnalysisOptionsYamlFile(testPackageRootPath, ''' +plugins: + solid_lints: + rules: + - feature_envy + diagnostics: + feature_envy: + atfd_threshold: 2 + laa_threshold: 0.33 + fdp_threshold: 1 +'''); + + await assertNoDiagnostics(r''' +class External { + void method1() {} + void method2() {} + void method3() {} +} + +class Notifier { + int val = 0; + void update(External ext) { + ext.method1(); + ext.method2(); + ext.method3(); + val = 1; + val = 2; + } +} +'''); + + await assertNoDiagnostics(r''' +class External1 { + void method1() {} + void method2() {} +} + +class External2 { + void method3() {} +} + +class Notifier { + void update(External1 ext1, External2 ext2) { + ext1.method1(); + ext1.method2(); + ext2.method3(); + } +} +'''); + } + + Future test_simple_assignment_does_not_double_count() => + assertNoDiagnostics(r''' +class External { + int value = 0; +} + +class MyClass { + void doSomething(External ext) { + ext.value = 5; + } +} +'''); + + Future test_does_not_report_on_data_class_accesses() => + assertNoDiagnostics(r''' +class DataClass { + final int a; + final int b; + const DataClass(this.a, this.b); + + @override + String toString() => '$a, $b'; + + @override + bool operator ==(Object other) => other is DataClass && other.a == a && other.b == b; + + @override + int get hashCode => Object.hash(a, b); +} + +class MyClass { + void doSomething(DataClass data) { + print(data.a); + print(data.b); + print(data.a); + print(data.b); + } +} +'''); + + Future test_reports_on_non_data_class_accesses() => + assertAutoDiagnostics(''' +class NormalClass { + final int a; + final int b; + const NormalClass(this.a, this.b); + + void customMethod() {} +} + +class MyClass { + void ${expectLint('doSomething')}(NormalClass normal) { + print(normal.a); + print(normal.b); + print(normal.a); + print(normal.b); + } +} +'''); + + Future + test_reports_envy_when_accesses_spread_across_multiple_classes() async { + await assertAutoDiagnostics(''' +class Circle { + final int radius; + const Circle(this.radius); + int get area => radius * radius; + void dummy() {} +} + +class Rectangle { + final int height; + final int width; + const Rectangle(this.height, this.width); + int get area => width * height; + void dummy() {} + void dummy2() {} +} + +class MathHelper { + int value = 0; + + int ${expectLint('sumOfAreas')}(Rectangle r, Circle c) { + value = 1; + return r.width + r.height + c.radius + c.area; + } +} +'''); + } + + Future test_implicit_extension_override_on_this_internal() => + assertNoDiagnostics(r''' +extension MyExtension on MathHelper { + void extMethod() {} +} + +class Rectangle { + final int width; + final int height; + const Rectangle(this.width, this.height); +} + +class MathHelper { + void doSomething(Rectangle r) { + extMethod(); + extMethod(); + print(r.width); + print(r.height); + } +} +'''); + + Future test_reports_intermediate_external_accesses_correctly() => + assertAutoDiagnostics(''' +class External1 { + final External2 ext2; + const External1(this.ext2); + void customMethod1() {} +} + +class External2 { + final int value; + const External2(this.value); + void customMethod2() {} +} + +class MathHelper { + void ${expectLint('doSomething', name: 'feature_envy', messageContainsAll: ['External1'])}(External1 ext1) { + print(ext1.ext2.value); + print(ext1.ext2.value); + } +} +'''); +} diff --git a/test/src/lints/feature_envy/models/feature_envy_metrics_test.dart b/test/src/lints/feature_envy/models/feature_envy_metrics_test.dart new file mode 100644 index 00000000..bbaeba3d --- /dev/null +++ b/test/src/lints/feature_envy/models/feature_envy_metrics_test.dart @@ -0,0 +1,78 @@ +import 'package:analyzer/dart/analysis/results.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer_testing/src/analysis_rule/pub_package_resolution.dart'; +import 'package:solid_lints/src/lints/feature_envy/models/feature_envy_metrics.dart'; +import 'package:test/test.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(FeatureEnvyMetricsTest); + }); +} + +@reflectiveTest +class FeatureEnvyMetricsTest extends PubPackageResolutionTest { + Future _resolveCode(String code) { + newFile(testFile.path, code); + return resolveFile(testFile.path); + } + + ClassElement _getClassElement(ResolvedUnitResult result, String className) { + final declaration = result.unit.declarations + .whereType() + .firstWhere((d) => d.namePart.typeName.lexeme == className); + return declaration.declaredFragment!.element; + } + + Future test_standard_calculations() async { + final resolved = await _resolveCode(''' +class ExternalA {} +'''); + + final extA = _getClassElement(resolved, 'ExternalA'); + + final metrics = FeatureEnvyMetrics.calculate( + internalAccesses: 1, + externalAccessCounts: {extA: 2}, + ); + + expect(metrics.laa, equals(1 / 3)); + expect(metrics.fdp, equals(1)); + expect(metrics.atfd, equals(2)); + expect(metrics.maxEnvyElement, equals(extA)); + } + + Future test_multiple_external_classes() async { + final resolved = await _resolveCode(''' +class ExternalA {} +class ExternalB {} +'''); + + final extA = _getClassElement(resolved, 'ExternalA'); + final extB = _getClassElement(resolved, 'ExternalB'); + + final metrics = FeatureEnvyMetrics.calculate( + internalAccesses: 2, + externalAccessCounts: {extA: 1, extB: 3}, + ); + + expect(metrics.laa, equals(2 / 6)); + expect(metrics.fdp, equals(2)); + expect(metrics.atfd, equals(3)); + expect(metrics.maxEnvyElement, equals(extB)); + } + + Future test_no_accesses_at_all() async { + final metrics = FeatureEnvyMetrics.calculate( + internalAccesses: 0, + externalAccessCounts: const {}, + ); + + expect(metrics.laa, equals(1.0)); + expect(metrics.fdp, equals(0)); + expect(metrics.atfd, equals(0)); + expect(metrics.maxEnvyElement, isNull); + } +} From 368dd8c5c82d25dbbe3c27a80ba5758ab94a3ef2 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Mon, 20 Jul 2026 19:47:37 +0300 Subject: [PATCH 2/9] refactor: simplify feature envy logic and improve code readability using pattern matching and collection methods --- .../models/feature_envy_metrics.dart | 24 ++-- .../models/feature_envy_parameters.dart | 32 ++---- .../utils/member_access_utils.dart | 107 +++++++----------- .../visitors/feature_envy_visitor.dart | 33 +++--- .../visitors/member_access_visitor.dart | 20 ++-- 5 files changed, 80 insertions(+), 136 deletions(-) diff --git a/lib/src/lints/feature_envy/models/feature_envy_metrics.dart b/lib/src/lints/feature_envy/models/feature_envy_metrics.dart index 79d3ccfa..6179a144 100644 --- a/lib/src/lints/feature_envy/models/feature_envy_metrics.dart +++ b/lib/src/lints/feature_envy/models/feature_envy_metrics.dart @@ -41,27 +41,17 @@ class FeatureEnvyMetrics { final laa = totalAccesses == 0 ? 1.0 : internalAccesses / totalAccesses; final fdp = externalAccessCounts.length; - int maxExternalCount = 0; - InterfaceElement? maxElement; - - for (final MapEntry(:key, :value) in externalAccessCounts.entries) { - final isLarger = value > maxExternalCount; - final isTieWithSmallerName = - value == maxExternalCount && - maxElement != null && - (key.name ?? '').compareTo(maxElement.name ?? '') < 0; - - if (isLarger || isTieWithSmallerName) { - maxExternalCount = value; - maxElement = key; - } - } + final maxEntry = externalAccessCounts.entries + .sorted((a, b) => b.value != a.value + ? b.value.compareTo(a.value) + : (a.key.name ?? '').compareTo(b.key.name ?? '')) + .firstOrNull; return FeatureEnvyMetrics._( laa: laa, fdp: fdp, - atfd: maxExternalCount, - maxEnvyElement: maxElement, + atfd: maxEntry?.value ?? 0, + maxEnvyElement: maxEntry?.key, ); } } diff --git a/lib/src/lints/feature_envy/models/feature_envy_parameters.dart b/lib/src/lints/feature_envy/models/feature_envy_parameters.dart index 49895aa8..a2b97195 100644 --- a/lib/src/lints/feature_envy/models/feature_envy_parameters.dart +++ b/lib/src/lints/feature_envy/models/feature_envy_parameters.dart @@ -43,28 +43,12 @@ class FeatureEnvyParameters { ); /// Creates a [FeatureEnvyParameters] model from JSON data. - factory FeatureEnvyParameters.fromJson(Map json) { - final atfdThreshold = switch (json['atfd_threshold']) { - final int value => value, - _ => _defaultAtfdThreshold, - }; - - final laaThreshold = switch (json['laa_threshold']) { - final double value => value, - final int value => value.toDouble(), - _ => _defaultLaaThreshold, - }; - - final fdpThreshold = switch (json['fdp_threshold']) { - final int value => value, - _ => _defaultFdpThreshold, - }; - - return FeatureEnvyParameters( - exclude: ExcludedIdentifiersListParameter.defaultFromJson(json), - atfdThreshold: atfdThreshold, - laaThreshold: laaThreshold, - fdpThreshold: fdpThreshold, - ); - } + factory FeatureEnvyParameters.fromJson(Map json) => + FeatureEnvyParameters( + exclude: ExcludedIdentifiersListParameter.defaultFromJson(json), + atfdThreshold: json['atfd_threshold'] as int? ?? _defaultAtfdThreshold, + laaThreshold: + (json['laa_threshold'] as num?)?.toDouble() ?? _defaultLaaThreshold, + fdpThreshold: json['fdp_threshold'] as int? ?? _defaultFdpThreshold, + ); } diff --git a/lib/src/lints/feature_envy/utils/member_access_utils.dart b/lib/src/lints/feature_envy/utils/member_access_utils.dart index 08a6cfa0..707a7153 100644 --- a/lib/src/lints/feature_envy/utils/member_access_utils.dart +++ b/lib/src/lints/feature_envy/utils/member_access_utils.dart @@ -11,23 +11,19 @@ abstract final class MemberAccessUtils { Expression? target, { required bool isPatternField, }) { - final unwrapped = target?.unwrapTarget; - final baseElement = switch (unwrapped) { - _ when isPatternField => null, - ExtensionOverride( - argumentList: ArgumentList(arguments: [final firstArg, ...]), - ) => - firstArg.staticType?.element, - final expr? => expr.staticType?.element, - _ => null, - }; + if (isPatternField || target == null) return null; - var resolved = baseElement?.resolveTypeParameter; - if (resolved case ExtensionElement(:final extendedType)) { - resolved = extendedType.element?.resolveTypeParameter; - } + final baseElement = switch (target.unwrapTarget) { + ExtensionOverride(:final argumentList) => + argumentList.arguments.firstOrNull?.staticType?.element, + final expr => expr?.staticType?.element, + }; - return resolved; + return switch (baseElement?.resolveTypeParameter) { + ExtensionElement(:final extendedType) => + extendedType.element?.resolveTypeParameter, + final resolved => resolved, + }; } /// Checks if the access to [c] is considered internal to [currentClass]. @@ -36,20 +32,15 @@ abstract final class MemberAccessUtils { InterfaceElement c, { required InterfaceElement currentClass, bool isPatternField = false, - }) { - if (!currentClass.isSameOrSubclassOf(c)) return false; - if (target == null && isPatternField) return false; - - final unwrapped = target?.unwrapTarget; - - return switch (unwrapped) { - ExtensionOverride( - argumentList: ArgumentList(arguments: [final firstArg, ...]), - ) => - firstArg.unwrapTarget.isThisOrSuper, - _ => unwrapped.isThisOrSuperOrNull, - }; - } + }) => + currentClass.isSameOrSubclassOf(c) && + (target != null || !isPatternField) && + switch (target?.unwrapTarget) { + ExtensionOverride(:final argumentList) => + argumentList.arguments.firstOrNull?.unwrapTarget.isThisOrSuper ?? + false, + final expr => expr.isThisOrSuperOrNull, + }; /// Finds the highest level access expression that starts with [node]. static Expression getAccessExpression(SimpleIdentifier node) => @@ -72,49 +63,30 @@ abstract final class MemberAccessUtils { required InterfaceElement currentClass, required ProjectClassCache projectClassCache, }) { - if (node.parent == null) return false; - - final element = node.element; - if (element == null) return false; - - final enclosing = element.enclosingInterface; + final enclosing = node.element?.enclosingInterface; if (enclosing == null || !currentClass.isSameOrSubclassOf(enclosing)) { return false; } final expr = getAccessExpression(node); - final exprParent = expr.parent; - if (exprParent == null) return false; - return switch (exprParent) { + bool isExternal(Element? e) => isExternalMember( + e, + currentClass: currentClass, + projectClassCache: projectClassCache, + ); + + return switch (expr.parent) { MethodInvocation(:final target, :final methodName) when target == expr => - isExternalMember( - methodName.element, - currentClass: currentClass, - projectClassCache: projectClassCache, - ), + isExternal(methodName.element), PrefixedIdentifier(:final prefix, :final identifier) when prefix == expr => - isExternalMember( - identifier.element, - currentClass: currentClass, - projectClassCache: projectClassCache, - ), + isExternal(identifier.element), PropertyAccess(:final target, :final propertyName) when target == expr => - isExternalMember( - propertyName.element, - currentClass: currentClass, - projectClassCache: projectClassCache, - ), + isExternal(propertyName.element), CascadeExpression(:final target, :final cascadeSections) when target == expr => - cascadeSections.any( - (s) => isExternalMember( - s.memberElement, - currentClass: currentClass, - projectClassCache: projectClassCache, - ), - ), + cascadeSections.any((s) => isExternal(s.memberElement)), _ => false, }; } @@ -124,12 +96,11 @@ abstract final class MemberAccessUtils { Element? element, { required InterfaceElement currentClass, required ProjectClassCache projectClassCache, - }) { - final enclosing = element?.enclosingInterface; - if (enclosing == null) return false; - - return !currentClass.isSameOrSubclassOf(enclosing) && - projectClassCache.isProjectClass(enclosing) && - !enclosing.isDataClass; - } + }) => switch (element?.enclosingInterface) { + final enclosing? => + !currentClass.isSameOrSubclassOf(enclosing) && + !enclosing.isDataClass && + projectClassCache.isProjectClass(enclosing), + _ => false, + }; } diff --git a/lib/src/lints/feature_envy/visitors/feature_envy_visitor.dart b/lib/src/lints/feature_envy/visitors/feature_envy_visitor.dart index 27fc53eb..6e836a97 100644 --- a/lib/src/lints/feature_envy/visitors/feature_envy_visitor.dart +++ b/lib/src/lints/feature_envy/visitors/feature_envy_visitor.dart @@ -19,15 +19,16 @@ class FeatureEnvyVisitor extends SimpleAstVisitor { @override void visitMethodDeclaration(MethodDeclaration node) { - if (node.isStatic) return; // ignore static methods - if (node.body case EmptyFunctionBody()) return; - if (_parameters.exclude.shouldIgnore(node)) return; + if (node.isStatic || + node.body is EmptyFunctionBody || + _parameters.exclude.shouldIgnore(node)) { + return; + } final interfaceElement = node.declaredFragment?.element.enclosingElement; - if (interfaceElement is! InterfaceElement) return; - - final classType = interfaceElement.thisType; - if (isWidgetOrSubclass(classType) || isWidgetStateOrSubclass(classType)) { + if (interfaceElement is! InterfaceElement || + isWidgetOrSubclass(interfaceElement.thisType) || + isWidgetStateOrSubclass(interfaceElement.thisType)) { return; } @@ -44,14 +45,14 @@ class FeatureEnvyVisitor extends SimpleAstVisitor { externalAccessCounts: accessVisitor.externalAccessCounts, ); - if (metrics.exceedsThresholds(_parameters)) { - _rule.reportAtToken( - node.name, - arguments: [ - metrics.maxEnvyElement?.name ?? '', - interfaceElement.name ?? '', - ], - ); - } + if (!metrics.exceedsThresholds(_parameters)) return; + + _rule.reportAtToken( + node.name, + arguments: [ + metrics.maxEnvyElement?.name ?? '', + interfaceElement.name ?? '', + ], + ); } } diff --git a/lib/src/lints/feature_envy/visitors/member_access_visitor.dart b/lib/src/lints/feature_envy/visitors/member_access_visitor.dart index e110802a..dcba78a1 100644 --- a/lib/src/lints/feature_envy/visitors/member_access_visitor.dart +++ b/lib/src/lints/feature_envy/visitors/member_access_visitor.dart @@ -25,18 +25,16 @@ class MemberAccessVisitor extends RecursiveAstVisitor { void visitSimpleIdentifier(SimpleIdentifier node) { super.visitSimpleIdentifier(node); - final element = node.element; - if (element == null || element is SetterElement) return; - - if (MemberAccessUtils.isTargetOfExternalAccess( - node, - currentClass: _currentClass, - projectClassCache: _projectClassCache, - )) { - return; - } + if (node.element case final element? + when element is! SetterElement && element.isInstanceMember) { + if (MemberAccessUtils.isTargetOfExternalAccess( + node, + currentClass: _currentClass, + projectClassCache: _projectClassCache, + )) { + return; + } - if (element.isInstanceMember) { _processElement( element: element, target: node.memberAccessTarget, From 488883212d3649911facd92d70f67b39877689b7 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 22 Jul 2026 11:52:27 +0300 Subject: [PATCH 3/9] refactor: simplify metrics calculation and use multiSortedBy for external access evaluation --- .../models/feature_envy_metrics.dart | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/src/lints/feature_envy/models/feature_envy_metrics.dart b/lib/src/lints/feature_envy/models/feature_envy_metrics.dart index 6179a144..8a4ea44e 100644 --- a/lib/src/lints/feature_envy/models/feature_envy_metrics.dart +++ b/lib/src/lints/feature_envy/models/feature_envy_metrics.dart @@ -1,6 +1,7 @@ import 'package:analyzer/dart/element/element.dart'; import 'package:collection/collection.dart'; import 'package:solid_lints/src/lints/feature_envy/models/feature_envy_parameters.dart'; +import 'package:solid_lints/src/utils/iterable_utils.dart'; /// Calculated Feature Envy metrics for a method. class FeatureEnvyMetrics { @@ -35,21 +36,15 @@ class FeatureEnvyMetrics { required int internalAccesses, required Map externalAccessCounts, }) { - final totalExternalAccesses = externalAccessCounts.values.sum; - final totalAccesses = internalAccesses + totalExternalAccesses; - - final laa = totalAccesses == 0 ? 1.0 : internalAccesses / totalAccesses; - final fdp = externalAccessCounts.length; - - final maxEntry = externalAccessCounts.entries - .sorted((a, b) => b.value != a.value - ? b.value.compareTo(a.value) - : (a.key.name ?? '').compareTo(b.key.name ?? '')) - .firstOrNull; + final totalAccesses = internalAccesses + externalAccessCounts.values.sum; + final maxEntry = externalAccessCounts.entries.multiSortedBy([ + (e) => -e.value, + (e) => e.key.name ?? '', + ]).firstOrNull; return FeatureEnvyMetrics._( - laa: laa, - fdp: fdp, + laa: totalAccesses == 0 ? 1.0 : internalAccesses / totalAccesses, + fdp: externalAccessCounts.length, atfd: maxEntry?.value ?? 0, maxEnvyElement: maxEntry?.key, ); From e2fdb640a19b73b5549531559d06052d59fbd455 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 22 Jul 2026 11:54:00 +0300 Subject: [PATCH 4/9] refactor: convert FeatureEnvyParameters.fromJson factory to a generative constructor --- .../models/feature_envy_parameters.dart | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/src/lints/feature_envy/models/feature_envy_parameters.dart b/lib/src/lints/feature_envy/models/feature_envy_parameters.dart index a2b97195..2b054dff 100644 --- a/lib/src/lints/feature_envy/models/feature_envy_parameters.dart +++ b/lib/src/lints/feature_envy/models/feature_envy_parameters.dart @@ -43,12 +43,10 @@ class FeatureEnvyParameters { ); /// Creates a [FeatureEnvyParameters] model from JSON data. - factory FeatureEnvyParameters.fromJson(Map json) => - FeatureEnvyParameters( - exclude: ExcludedIdentifiersListParameter.defaultFromJson(json), - atfdThreshold: json['atfd_threshold'] as int? ?? _defaultAtfdThreshold, - laaThreshold: - (json['laa_threshold'] as num?)?.toDouble() ?? _defaultLaaThreshold, - fdpThreshold: json['fdp_threshold'] as int? ?? _defaultFdpThreshold, - ); + FeatureEnvyParameters.fromJson(Map json) + : exclude = ExcludedIdentifiersListParameter.defaultFromJson(json), + atfdThreshold = json['atfd_threshold'] as int? ?? _defaultAtfdThreshold, + laaThreshold = + (json['laa_threshold'] as num?)?.toDouble() ?? _defaultLaaThreshold, + fdpThreshold = json['fdp_threshold'] as int? ?? _defaultFdpThreshold; } From fbfd60e1c064b303c11c8d55323efa775bbd408d Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 22 Jul 2026 12:18:32 +0300 Subject: [PATCH 5/9] refactor: simplify public member extraction using extension methods and set operations --- lib/src/utils/types_utils.dart | 48 +++++++++++++++++----------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/lib/src/utils/types_utils.dart b/lib/src/utils/types_utils.dart index 33878070..2a4fe6bf 100644 --- a/lib/src/utils/types_utils.dart +++ b/lib/src/utils/types_utils.dart @@ -88,6 +88,11 @@ extension _PropertyAccessorElementExt on PropertyAccessorElement { isPublic && !isStatic && isOriginDeclaration; } +extension _PropertyAccessorsExt on Iterable { + Iterable get publicInstanceMemberNames => + where((e) => e.isPublicInstanceOrigin).map((e) => e.displayName); +} + extension InterfaceElementExt on InterfaceElement { /// Checks whether this element is the same as or a subclass of [superType]. bool isSameOrSubclassOf(InterfaceElement superType) => @@ -103,34 +108,29 @@ extension InterfaceElementExt on InterfaceElement { ...allSupertypes.where((s) => !s.isDartCoreObject).map((s) => s.element), ]; - final publicProperties = { + final publicProperties = { for (final el in elements) ...[ - ...el.getters - .where((g) => g.isPublicInstanceOrigin) - .map((g) => g.displayName) - .where((name) => !const {'hashCode', 'runtimeType'}.contains(name)), - ...el.setters - .where((s) => s.isPublicInstanceOrigin) - .map((s) => s.displayName), + ...el.getters.publicInstanceMemberNames.toSet().difference({ + 'hashCode', + 'runtimeType', + }), + ...el.setters.publicInstanceMemberNames, ], }; - final publicMethods = { - for (final el in elements) - ...el.methods - .where((m) => m.isPublic && !m.isStatic) - .map((m) => m.name) - .nonNulls - .where( - (name) => !const { - 'toString', - '==', - 'copyWith', - 'toJson', - 'noSuchMethod', - }.contains(name), - ), - }; + final publicMethods = elements + .expand((e) => e.methods) + .where((m) => m.isPublic && !m.isStatic) + .map((m) => m.name) + .nonNulls + .toSet() + .difference({ + 'toString', + '==', + 'copyWith', + 'toJson', + 'noSuchMethod', + }); final totalMembers = publicProperties.length + publicMethods.length; if (totalMembers == 0) return true; From b6a5cdc3b84d22c4372fd3659f2eb6afc228e4ae Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 22 Jul 2026 12:26:29 +0300 Subject: [PATCH 6/9] refactor: simplify instance member checks, element traversal, and member element resolution logic --- lib/src/utils/node_utils.dart | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/src/utils/node_utils.dart b/lib/src/utils/node_utils.dart index f509b868..15562947 100644 --- a/lib/src/utils/node_utils.dart +++ b/lib/src/utils/node_utils.dart @@ -190,8 +190,8 @@ extension ElementExtension on Element { /// Checks whether this element is a non-static instance member. bool get isInstanceMember => switch (this) { - PropertyAccessorElement(:final isStatic) => !isStatic, - MethodElement(:final isStatic) => !isStatic, + PropertyAccessorElement(:final isStatic) || + MethodElement(:final isStatic) || FieldElement(:final isStatic) => !isStatic, _ => false, }; @@ -212,15 +212,14 @@ extension ElementExtension on Element { /// Returns the enclosing [InterfaceElement] of this element (recursively), /// or null if none. - InterfaceElement? get enclosingInterface { - Element? current = this; - while (current != null) { - if (current case InterfaceElement()) { - return current; - } - current = current.enclosingElement; + InterfaceElement? get enclosingInterface => + enclosingElements.whereType().firstOrNull; + + /// Returns an iterable of this element and all its enclosing elements. + Iterable get enclosingElements sync* { + for (Element? e = this; e != null; e = e.enclosingElement) { + yield e; } - return null; } /// Resolves type parameters to their bounds recursively to prevent cycle @@ -276,12 +275,12 @@ extension ExpressionExtension on Expression { Element? get memberElement => switch (this) { MethodInvocation(:final methodName) => methodName.element, PropertyAccess(:final propertyName) => propertyName.element, - AssignmentExpression(:final writeElement, :final readElement) => - writeElement ?? readElement, - PrefixExpression(:final writeElement, :final readElement) => - writeElement ?? readElement, - PostfixExpression(:final writeElement, :final readElement) => - writeElement ?? readElement, + AssignmentExpression(:final writeElement, :final readElement) || + PostfixExpression(:final writeElement, :final readElement) || + PrefixExpression( + :final writeElement, + :final readElement, + ) => writeElement ?? readElement, IndexExpression(:final element) => element, BinaryExpression(:final element) => element, _ => null, From d5390ce1610e9336a0528d720fdceeee525283af Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 22 Jul 2026 12:35:12 +0300 Subject: [PATCH 7/9] refactor: simplify test code in IsDataClassTest by introducing a shared _test helper method --- test/src/common/utils/is_data_class_test.dart | 64 ++++++++----------- 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/test/src/common/utils/is_data_class_test.dart b/test/src/common/utils/is_data_class_test.dart index 9ff01f5a..4d853ae7 100644 --- a/test/src/common/utils/is_data_class_test.dart +++ b/test/src/common/utils/is_data_class_test.dart @@ -1,4 +1,3 @@ -import 'package:analyzer/dart/analysis/results.dart'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer_testing/src/analysis_rule/pub_package_resolution.dart'; import 'package:solid_lints/src/utils/types_utils.dart'; @@ -13,30 +12,15 @@ void main() { @reflectiveTest class IsDataClassTest extends PubPackageResolutionTest { - Future _resolveCode(String code) async { - newFile(testFile.path, code); - return resolveFile(testFile.path); - } - - Future test_isDataClass_for_pure_data_class() async { - final resolved = await _resolveCode(''' + Future test_isDataClass_for_pure_data_class() async => _test(''' class Rectangle { final int width; final int height; const Rectangle(this.width, this.height); } -'''); - final classDecl = resolved.unit.declarations - .whereType() - .first; - final element = classDecl.declaredFragment?.element; - expect(element, isNotNull); - - expect(element?.isDataClass, isTrue); - } +''', isTrue); - Future test_isDataClass_for_non_data_class() async { - final resolved = await _resolveCode(''' + Future test_isDataClass_for_non_data_class() async => _test(''' class Rectangle { final int width; final int height; @@ -44,18 +28,10 @@ class Rectangle { int area() => width * height; void printMe() {} } -'''); - final classDecl = resolved.unit.declarations - .whereType() - .first; - final element = classDecl.declaredFragment?.element; - expect(element, isNotNull); +''', isFalse); - expect(element?.isDataClass, isFalse); - } - - Future test_isDataClass_collision_case() async { - final resolved = await _resolveCode(''' + Future test_isDataClass_collision_case() async => _test( + ''' class Base { int get foo => 42; } @@ -64,13 +40,27 @@ class Sub extends Base { @override int foo() => 42; } -'''); - final subDecl = resolved.unit.declarations - .whereType() - .firstWhere((d) => d.namePart.typeName.lexeme == 'Sub'); - final element = subDecl.declaredFragment?.element; - expect(element, isNotNull); +''', + isFalse, + (d) => d.namePart.typeName.lexeme == 'Sub', + ); + + Future _test( + String source, + dynamic matcher, [ + bool Function(ClassDeclaration)? test, + ]) async { + newFile(testFile.path, source); + final file = await resolveFile(testFile.path); - expect(element?.isDataClass, isFalse); + expect( + file.unit.declarations + .whereType() + .firstWhere(test ?? (_) => true) + .declaredFragment + ?.element + .isDataClass, + matcher, + ); } } From 00b3c548c37eab509dc900a1b71b8703f08f4131 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 22 Jul 2026 12:40:19 +0300 Subject: [PATCH 8/9] refactor: introduce _expectMetrics helper to simplify feature envy metrics unit tests --- .../models/feature_envy_metrics_test.dart | 50 ++++++++++++------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/test/src/lints/feature_envy/models/feature_envy_metrics_test.dart b/test/src/lints/feature_envy/models/feature_envy_metrics_test.dart index bbaeba3d..8786d7dd 100644 --- a/test/src/lints/feature_envy/models/feature_envy_metrics_test.dart +++ b/test/src/lints/feature_envy/models/feature_envy_metrics_test.dart @@ -33,15 +33,14 @@ class ExternalA {} final extA = _getClassElement(resolved, 'ExternalA'); - final metrics = FeatureEnvyMetrics.calculate( + _expectMetrics( internalAccesses: 1, externalAccessCounts: {extA: 2}, + laa: 1 / 3, + fdp: 1, + atfd: 2, + maxEnvyElement: extA, ); - - expect(metrics.laa, equals(1 / 3)); - expect(metrics.fdp, equals(1)); - expect(metrics.atfd, equals(2)); - expect(metrics.maxEnvyElement, equals(extA)); } Future test_multiple_external_classes() async { @@ -53,26 +52,43 @@ class ExternalB {} final extA = _getClassElement(resolved, 'ExternalA'); final extB = _getClassElement(resolved, 'ExternalB'); - final metrics = FeatureEnvyMetrics.calculate( + _expectMetrics( internalAccesses: 2, externalAccessCounts: {extA: 1, extB: 3}, + laa: 2 / 6, + fdp: 2, + atfd: 3, + maxEnvyElement: extB, ); - - expect(metrics.laa, equals(2 / 6)); - expect(metrics.fdp, equals(2)); - expect(metrics.atfd, equals(3)); - expect(metrics.maxEnvyElement, equals(extB)); } Future test_no_accesses_at_all() async { - final metrics = FeatureEnvyMetrics.calculate( + _expectMetrics( internalAccesses: 0, externalAccessCounts: const {}, + laa: 1.0, + fdp: 0, + atfd: 0, + maxEnvyElement: null, + ); + } + + void _expectMetrics({ + required int internalAccesses, + required Map externalAccessCounts, + required double laa, + required int fdp, + required int atfd, + required InterfaceElement? maxEnvyElement, + }) { + final metrics = FeatureEnvyMetrics.calculate( + internalAccesses: internalAccesses, + externalAccessCounts: externalAccessCounts, ); - expect(metrics.laa, equals(1.0)); - expect(metrics.fdp, equals(0)); - expect(metrics.atfd, equals(0)); - expect(metrics.maxEnvyElement, isNull); + expect(metrics.laa, equals(laa)); + expect(metrics.fdp, equals(fdp)); + expect(metrics.atfd, equals(atfd)); + expect(metrics.maxEnvyElement, equals(maxEnvyElement)); } } From 4bb4a6d7d90a250e4fe3243026b5f5b0a3109573 Mon Sep 17 00:00:00 2001 From: Illia Aihistov Date: Wed, 22 Jul 2026 16:43:05 +0300 Subject: [PATCH 9/9] refactor: reorder constructor parameters and convert empty factory to generative constructor for FeatureEnvyParameters --- .../models/feature_envy_parameters.dart | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/src/lints/feature_envy/models/feature_envy_parameters.dart b/lib/src/lints/feature_envy/models/feature_envy_parameters.dart index 2b054dff..01686bba 100644 --- a/lib/src/lints/feature_envy/models/feature_envy_parameters.dart +++ b/lib/src/lints/feature_envy/models/feature_envy_parameters.dart @@ -28,19 +28,18 @@ class FeatureEnvyParameters { /// Constructor for [FeatureEnvyParameters] model. const FeatureEnvyParameters({ - required this.atfdThreshold, required this.exclude, + required this.atfdThreshold, required this.laaThreshold, required this.fdpThreshold, }); /// Empty [FeatureEnvyParameters] model with default values. - factory FeatureEnvyParameters.empty() => FeatureEnvyParameters( - atfdThreshold: _defaultAtfdThreshold, - exclude: ExcludedIdentifiersListParameter(exclude: []), - laaThreshold: _defaultLaaThreshold, - fdpThreshold: _defaultFdpThreshold, - ); + FeatureEnvyParameters.empty() + : exclude = ExcludedIdentifiersListParameter(exclude: []), + atfdThreshold = _defaultAtfdThreshold, + laaThreshold = _defaultLaaThreshold, + fdpThreshold = _defaultFdpThreshold; /// Creates a [FeatureEnvyParameters] model from JSON data. FeatureEnvyParameters.fromJson(Map json)