diff --git a/.github/workflows/lint_pr_title.yaml b/.github/workflows/lint_pr_title.yaml index 17d7c63867..dca8d23a14 100644 --- a/.github/workflows/lint_pr_title.yaml +++ b/.github/workflows/lint_pr_title.yaml @@ -8,6 +8,7 @@ on: jobs: conventional_pr_title: + name: Conventional PR title runs-on: ubuntu-latest steps: - uses: amannn/action-semantic-pull-request@v3.4.2 @@ -18,6 +19,7 @@ jobs: # See https://github.com/GetStream/verify-semantic-changelog-update semantic_changelog_update: + name: Semantic changelog update needs: conventional_pr_title # Trigger after the [conventional_pr_title] completes runs-on: ubuntu-latest steps: diff --git a/lib/src/analyzer_plugin/analyzer_plugin.dart b/lib/src/analyzer_plugin/analyzer_plugin.dart index a010e399d8..cda2440b53 100644 --- a/lib/src/analyzer_plugin/analyzer_plugin.dart +++ b/lib/src/analyzer_plugin/analyzer_plugin.dart @@ -15,8 +15,8 @@ import 'package:analyzer_plugin/protocol/protocol_generated.dart' as plugin; import '../analyzers/lint_analyzer/lint_analysis_config.dart'; import '../analyzers/lint_analyzer/lint_analyzer.dart'; -import '../analyzers/lint_analyzer/metrics/metrics_list/cyclomatic_complexity/cyclomatic_complexity_metric.dart'; import '../analyzers/lint_analyzer/metrics/metrics_list/number_of_parameters_metric.dart'; +import '../analyzers/lint_analyzer/metrics/metrics_list/source_lines_of_code/source_lines_of_code_metric.dart'; import '../config_builder/config_builder.dart'; import '../config_builder/models/analysis_options.dart'; import '../utils/yaml_utils.dart'; @@ -246,8 +246,8 @@ class MetricsAnalyzerPlugin extends ServerPlugin { options.folderPath ?? rootPath, classMetrics: const [], functionMetrics: [ - CyclomaticComplexityMetric(config: config.metrics), NumberOfParametersMetric(config: config.metrics), + SourceLinesOfCodeMetric(config: config.metrics), ], ); diff --git a/lib/src/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_method.dart b/lib/src/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_method.dart index a8a17791d5..bf273b9a39 100644 --- a/lib/src/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_method.dart +++ b/lib/src/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_method.dart @@ -19,8 +19,20 @@ import '../pattern_utils.dart'; class LongMethod extends ObsoletePattern { static const String patternId = 'long-method'; - LongMethod([Map config = const {}]) - : super( + final int _sourceLinesOfCodeMetricTreshold; + + @override + Iterable get dependentMetricIds => [SourceLinesOfCodeMetric.metricId]; + + LongMethod({ + Map patternSettings = const {}, + Map metricstTresholds = const {}, + }) : _sourceLinesOfCodeMetricTreshold = readThreshold( + metricstTresholds, + SourceLinesOfCodeMetric.metricId, + 50, + ), + super( id: patternId, documentation: const PatternDocumentation( name: 'Long Method', @@ -28,44 +40,39 @@ class LongMethod extends ObsoletePattern { 'Long blocks of code are difficult to reuse and understand because they are usually responsible for more than one thing. Separating those to several short ones with proper names helps you reuse your code and understand it better without reading methods body.', supportedType: EntityType.methodEntity, ), - severity: readSeverity(config, Severity.none), + severity: readSeverity(patternSettings, Severity.none), ); @override Iterable legacyCheck( InternalResolvedUnitResult source, Iterable functions, - Map metricsConfig, - ) { - final threshold = - readThreshold(metricsConfig, SourceLinesOfCodeMetric.metricId, 50); - - return functions - .where((function) => !_isExcluded(function)) - .expand((function) { - final visitor = SourceCodeVisitor(source.lineInfo); - function.declaration.visitChildren(visitor); + ) => + functions + .where((function) => !_isExcluded(function)) + .expand((function) { + final visitor = SourceCodeVisitor(source.lineInfo); + function.declaration.visitChildren(visitor); - return [ - if (visitor.linesWithCode.length > threshold) - createIssue( - pattern: this, - location: nodeLocation( - node: function.declaration, - source: source, + return [ + if (visitor.linesWithCode.length > _sourceLinesOfCodeMetricTreshold) + createIssue( + pattern: this, + location: nodeLocation( + node: function.declaration, + source: source, + ), + message: _compileMessage( + lines: visitor.linesWithCode.length, + functionType: function.type, + ), + verboseMessage: _compileRecommendationMessage( + maximumLines: _sourceLinesOfCodeMetricTreshold, + functionType: function.type, + ), ), - message: _compileMessage( - lines: visitor.linesWithCode.length, - functionType: function.type, - ), - verboseMessage: _compileRecommendationMessage( - maximumLines: threshold, - functionType: function.type, - ), - ), - ]; - }).toList(); - } + ]; + }).toList(); bool _isExcluded(ScopedFunctionDeclaration function) { final declaration = function.declaration; diff --git a/lib/src/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_parameter_list.dart b/lib/src/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_parameter_list.dart index 24bd134f07..068221ec6c 100644 --- a/lib/src/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_parameter_list.dart +++ b/lib/src/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_parameter_list.dart @@ -15,8 +15,21 @@ import '../pattern_utils.dart'; class LongParameterList extends ObsoletePattern { static const String patternId = 'long-parameter-list'; - LongParameterList([Map config = const {}]) - : super( + final int _numberOfParametersMetricTreshold; + + @override + Iterable get dependentMetricIds => + [NumberOfParametersMetric.metricId]; + + LongParameterList({ + Map patternSettings = const {}, + Map metricstTresholds = const {}, + }) : _numberOfParametersMetricTreshold = readThreshold( + metricstTresholds, + NumberOfParametersMetric.metricId, + 4, + ), + super( id: patternId, documentation: const PatternDocumentation( name: 'Long Parameter List', @@ -24,37 +37,33 @@ class LongParameterList extends ObsoletePattern { 'Long parameter lists are difficult to understand and use. Wrapping them into an object allows grouping parameters and change transferred data only by the object modification.', supportedType: EntityType.methodEntity, ), - severity: readSeverity(config, Severity.none), + severity: readSeverity(patternSettings, Severity.none), ); @override Iterable legacyCheck( InternalResolvedUnitResult source, Iterable functions, - Map metricsConfig, - ) { - final threshold = - readThreshold(metricsConfig, NumberOfParametersMetric.metricId, 4); - - return functions - .where((function) => getArgumentsCount(function) > threshold) - .map((function) => createIssue( - pattern: this, - location: nodeLocation( - node: function.declaration, - source: source, - ), - message: _compileMessage( - args: getArgumentsCount(function), - functionType: function.type, - ), - verboseMessage: _compileRecommendationMessage( - maximumArguments: threshold, - functionType: function.type, - ), - )) - .toList(); - } + ) => + functions + .where((function) => + getArgumentsCount(function) > _numberOfParametersMetricTreshold) + .map((function) => createIssue( + pattern: this, + location: nodeLocation( + node: function.declaration, + source: source, + ), + message: _compileMessage( + args: getArgumentsCount(function), + functionType: function.type, + ), + verboseMessage: _compileRecommendationMessage( + maximumArguments: _numberOfParametersMetricTreshold, + functionType: function.type, + ), + )) + .toList(); String _compileMessage({ required int args, diff --git a/lib/src/analyzers/lint_analyzer/anti_patterns/models/obsolete_pattern.dart b/lib/src/analyzers/lint_analyzer/anti_patterns/models/obsolete_pattern.dart index db9b2e2f2e..264bb0ca3f 100644 --- a/lib/src/analyzers/lint_analyzer/anti_patterns/models/obsolete_pattern.dart +++ b/lib/src/analyzers/lint_analyzer/anti_patterns/models/obsolete_pattern.dart @@ -20,7 +20,6 @@ abstract class ObsoletePattern extends Pattern { Iterable legacyCheck( InternalResolvedUnitResult source, Iterable functions, - Map metricsConfig, ); @override diff --git a/lib/src/analyzers/lint_analyzer/anti_patterns/models/pattern.dart b/lib/src/analyzers/lint_analyzer/anti_patterns/models/pattern.dart index ce6af4350d..1be37b8117 100644 --- a/lib/src/analyzers/lint_analyzer/anti_patterns/models/pattern.dart +++ b/lib/src/analyzers/lint_analyzer/anti_patterns/models/pattern.dart @@ -11,18 +11,21 @@ abstract class Pattern { /// The id of the pattern. final String id; - /// The documentation associated with the pattern + /// The documentation associated with the pattern. final PatternDocumentation documentation; - /// The severity of issues emitted by the pattern + /// The severity of issues emitted by the pattern. final Severity severity; + /// Metric ids which values are used by the anti-pattern to detect a violation. + Iterable get dependentMetricIds; + const Pattern({ required this.id, required this.documentation, required this.severity, }); - /// Returns [Iterable] with [Issue]'s detected while check the passed [source] + /// Returns [Iterable] with [Issue]'s detected while check the passed [source]. Iterable check(InternalResolvedUnitResult source, Report report); } diff --git a/lib/src/analyzers/lint_analyzer/anti_patterns/patterns_factory.dart b/lib/src/analyzers/lint_analyzer/anti_patterns/patterns_factory.dart index 1a583f91cd..0c9ee34393 100644 --- a/lib/src/analyzers/lint_analyzer/anti_patterns/patterns_factory.dart +++ b/lib/src/analyzers/lint_analyzer/anti_patterns/patterns_factory.dart @@ -1,19 +1,29 @@ +import '../lint_config.dart'; import 'anti_patterns_list/long_method.dart'; import 'anti_patterns_list/long_parameter_list.dart'; import 'models/obsolete_pattern.dart'; -final _implementedPatterns = - )>{ - LongMethod.patternId: (config) => LongMethod(config), - LongParameterList.patternId: (config) => LongParameterList(config), +typedef CreatePattern = ObsoletePattern Function( + Map patternSettings, + Map metricstTresholds, +); + +final _implementedPatterns = { + LongMethod.patternId: (settings, tresholds) => + LongMethod(patternSettings: settings, metricstTresholds: tresholds), + LongParameterList.patternId: (settings, tresholds) => LongParameterList( + patternSettings: settings, + metricstTresholds: tresholds, + ), }; Iterable get allPatterns => - _implementedPatterns.keys.map((id) => _implementedPatterns[id]!({})); + _implementedPatterns.keys.map((id) => _implementedPatterns[id]!({}, {})); -Iterable getPatternsById(Map patternsConfig) => +Iterable getPatternsById(LintConfig config) => List.unmodifiable(_implementedPatterns.keys - .where((id) => patternsConfig.keys.contains(id)) + .where((id) => config.antiPatterns.keys.contains(id)) .map((id) => _implementedPatterns[id]!( - patternsConfig[id] as Map, + config.antiPatterns[id] as Map, + config.metrics, ))); diff --git a/lib/src/analyzers/lint_analyzer/lint_analyzer.dart b/lib/src/analyzers/lint_analyzer/lint_analyzer.dart index b2d2428fcb..d224e84059 100644 --- a/lib/src/analyzers/lint_analyzer/lint_analyzer.dart +++ b/lib/src/analyzers/lint_analyzer/lint_analyzer.dart @@ -236,7 +236,7 @@ class LintAnalyzer { config.antiPatterns .where((pattern) => !ignores.isSuppressed(pattern.id)) .expand((pattern) => pattern - .legacyCheck(source, functions, config.metricsConfig) + .legacyCheck(source, functions) .where((issue) => !ignores.isSuppressedAt( issue.ruleId, issue.location.start.line, diff --git a/lib/src/config_builder/config_builder.dart b/lib/src/config_builder/config_builder.dart index 9e455b9a52..c40e23321b 100644 --- a/lib/src/config_builder/config_builder.dart +++ b/lib/src/config_builder/config_builder.dart @@ -28,7 +28,7 @@ class ConfigBuilder { prepareExcludes(config.excludePatterns, excludesRootFolder), getRulesById(config.rules), prepareExcludes(config.excludeForRulesPatterns, excludesRootFolder), - getPatternsById(config.antiPatterns), + getPatternsById(config), classMetrics ?? getMetrics( config: config.metrics, diff --git a/test/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_method/long_method_test.dart b/test/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_method/long_method_test.dart index 35759ef3b1..95e1c51079 100644 --- a/test/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_method/long_method_test.dart +++ b/test/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_method/long_method_test.dart @@ -19,7 +19,9 @@ void main() { final scopeVisitor = ScopeVisitor(); unit.unit.visitChildren(scopeVisitor); - final issues = LongMethod().legacyCheck( + final issues = LongMethod( + metricstTresholds: {SourceLinesOfCodeMetric.metricId: 25}, + ).legacyCheck( unit, scopeVisitor.functions.where((function) { final declaration = function.declaration; @@ -33,7 +35,6 @@ void main() { return true; }), - {SourceLinesOfCodeMetric.metricId: 25}, ); AntiPatternTestHelper.verifyInitialization( @@ -64,7 +65,9 @@ void main() { final scopeVisitor = ScopeVisitor(); unit.unit.visitChildren(scopeVisitor); - final issues = LongMethod().legacyCheck( + final issues = LongMethod( + metricstTresholds: {SourceLinesOfCodeMetric.metricId: 25}, + ).legacyCheck( unit, scopeVisitor.functions.where((function) { final declaration = function.declaration; @@ -78,7 +81,6 @@ void main() { return true; }), - {SourceLinesOfCodeMetric.metricId: 25}, ); expect(issues, isEmpty); diff --git a/test/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_parameter_list/long_parameter_list_test.dart b/test/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_parameter_list/long_parameter_list_test.dart index ee154501de..8d26059a09 100644 --- a/test/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_parameter_list/long_parameter_list_test.dart +++ b/test/analyzers/lint_analyzer/anti_patterns/anti_patterns_list/long_parameter_list/long_parameter_list_test.dart @@ -17,7 +17,9 @@ void main() { final scopeVisitor = ScopeVisitor(); unit.unit.visitChildren(scopeVisitor); - final issues = LongParameterList().legacyCheck( + final issues = LongParameterList( + metricstTresholds: {NumberOfParametersMetric.metricId: 4}, + ).legacyCheck( unit, scopeVisitor.functions.where((function) { final declaration = function.declaration; @@ -31,7 +33,6 @@ void main() { return true; }), - {NumberOfParametersMetric.metricId: 4}, ); AntiPatternTestHelper.verifyInitialization( diff --git a/test/analyzers/lint_analyzer/anti_patterns/patterns_factory_test.dart b/test/analyzers/lint_analyzer/anti_patterns/patterns_factory_test.dart index bd8d67e905..31d1b8595c 100644 --- a/test/analyzers/lint_analyzer/anti_patterns/patterns_factory_test.dart +++ b/test/analyzers/lint_analyzer/anti_patterns/patterns_factory_test.dart @@ -1,16 +1,34 @@ @TestOn('vm') import 'package:dart_code_metrics/src/analyzers/lint_analyzer/anti_patterns/patterns_factory.dart'; +import 'package:dart_code_metrics/src/analyzers/lint_analyzer/lint_config.dart'; import 'package:test/test.dart'; void main() { test('getPatternsById returns only required patterns', () { - expect(getPatternsById({}), isEmpty); expect( - getPatternsById({ - 'long-method': {}, - 'long-parameter-list': {}, - 'sample-pattern': {}, - }).map((pattern) => pattern.id), + getPatternsById(const LintConfig( + excludePatterns: [], + excludeForMetricsPatterns: [], + metrics: {}, + rules: {}, + excludeForRulesPatterns: [], + antiPatterns: {}, + )), + isEmpty, + ); + expect( + getPatternsById(const LintConfig( + excludePatterns: [], + excludeForMetricsPatterns: [], + metrics: {}, + rules: {}, + excludeForRulesPatterns: [], + antiPatterns: { + 'long-method': {}, + 'long-parameter-list': {}, + 'sample-pattern': {}, + }, + )).map((pattern) => pattern.id), equals(['long-method', 'long-parameter-list']), ); });