Skip to content
This repository was archived by the owner on Jul 16, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/lint_pr_title.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions lib/src/analyzer_plugin/analyzer_plugin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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),
],
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,53 +19,60 @@ import '../pattern_utils.dart';
class LongMethod extends ObsoletePattern {
static const String patternId = 'long-method';

LongMethod([Map<String, Object> config = const {}])
: super(
final int _sourceLinesOfCodeMetricTreshold;

@override
Iterable<String> get dependentMetricIds => [SourceLinesOfCodeMetric.metricId];

LongMethod({
Map<String, Object> patternSettings = const {},
Map<String, Object> metricstTresholds = const {},
}) : _sourceLinesOfCodeMetricTreshold = readThreshold<int>(
metricstTresholds,
SourceLinesOfCodeMetric.metricId,
50,
),
super(
id: patternId,
documentation: const PatternDocumentation(
name: 'Long Method',
brief:
'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<Issue> legacyCheck(
InternalResolvedUnitResult source,
Iterable<ScopedFunctionDeclaration> functions,
Map<String, Object> metricsConfig,
) {
final threshold =
readThreshold<int>(metricsConfig, SourceLinesOfCodeMetric.metricId, 50);

return functions
.where((function) => !_isExcluded(function))
.expand<Issue>((function) {
final visitor = SourceCodeVisitor(source.lineInfo);
function.declaration.visitChildren(visitor);
) =>
functions
.where((function) => !_isExcluded(function))
.expand<Issue>((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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,46 +15,55 @@ import '../pattern_utils.dart';
class LongParameterList extends ObsoletePattern {
static const String patternId = 'long-parameter-list';

LongParameterList([Map<String, Object> config = const {}])
: super(
final int _numberOfParametersMetricTreshold;

@override
Iterable<String> get dependentMetricIds =>
[NumberOfParametersMetric.metricId];

LongParameterList({
Map<String, Object> patternSettings = const {},
Map<String, Object> metricstTresholds = const {},
}) : _numberOfParametersMetricTreshold = readThreshold<int>(
metricstTresholds,
NumberOfParametersMetric.metricId,
4,
),
super(
id: patternId,
documentation: const PatternDocumentation(
name: 'Long Parameter List',
brief:
'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<Issue> legacyCheck(
InternalResolvedUnitResult source,
Iterable<ScopedFunctionDeclaration> functions,
Map<String, Object> metricsConfig,
) {
final threshold =
readThreshold<int>(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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ abstract class ObsoletePattern extends Pattern {
Iterable<Issue> legacyCheck(
InternalResolvedUnitResult source,
Iterable<ScopedFunctionDeclaration> functions,
Map<String, Object> metricsConfig,
);

@override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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<Issue> check(InternalResolvedUnitResult source, Report report);
}
Original file line number Diff line number Diff line change
@@ -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 =
<String, ObsoletePattern Function(Map<String, Object>)>{
LongMethod.patternId: (config) => LongMethod(config),
LongParameterList.patternId: (config) => LongParameterList(config),
typedef CreatePattern = ObsoletePattern Function(
Map<String, Object> patternSettings,
Map<String, Object> metricstTresholds,
);

final _implementedPatterns = <String, CreatePattern>{
LongMethod.patternId: (settings, tresholds) =>
LongMethod(patternSettings: settings, metricstTresholds: tresholds),
LongParameterList.patternId: (settings, tresholds) => LongParameterList(
patternSettings: settings,
metricstTresholds: tresholds,
),
};

Iterable<ObsoletePattern> get allPatterns =>
_implementedPatterns.keys.map((id) => _implementedPatterns[id]!({}));
_implementedPatterns.keys.map((id) => _implementedPatterns[id]!({}, {}));

Iterable<ObsoletePattern> getPatternsById(Map<String, Object> patternsConfig) =>
Iterable<ObsoletePattern> getPatternsById(LintConfig config) =>
List.unmodifiable(_implementedPatterns.keys
.where((id) => patternsConfig.keys.contains(id))
.where((id) => config.antiPatterns.keys.contains(id))
.map<ObsoletePattern>((id) => _implementedPatterns[id]!(
patternsConfig[id] as Map<String, Object>,
config.antiPatterns[id] as Map<String, Object>,
config.metrics,
)));
2 changes: 1 addition & 1 deletion lib/src/analyzers/lint_analyzer/lint_analyzer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lib/src/config_builder/config_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,7 +35,6 @@ void main() {

return true;
}),
{SourceLinesOfCodeMetric.metricId: 25},
);

AntiPatternTestHelper.verifyInitialization(
Expand Down Expand Up @@ -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;
Expand All @@ -78,7 +81,6 @@ void main() {

return true;
}),
{SourceLinesOfCodeMetric.metricId: 25},
);

expect(issues, isEmpty);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -31,7 +33,6 @@ void main() {

return true;
}),
{NumberOfParametersMetric.metricId: 4},
);

AntiPatternTestHelper.verifyInitialization(
Expand Down
Original file line number Diff line number Diff line change
@@ -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': <String, Object>{},
'long-parameter-list': <String, Object>{},
'sample-pattern': <String, Object>{},
}).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']),
);
});
Expand Down