Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

Commit

Permalink
Merge 554657b into 28eea04
Browse files Browse the repository at this point in the history
  • Loading branch information
dkrutskikh committed Aug 30, 2020
2 parents 28eea04 + 554657b commit 4e66521
Show file tree
Hide file tree
Showing 25 changed files with 720 additions and 167 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ If you want command line tool to check rules, you should add configuration to yo
### Library
[See `example/example.dart`](https://github.com/wrike/dart-code-metrics/blob/master/example/example.dart)
## Anti-Patterns
* [long-method](https://github.com/wrike/dart-code-metrics/blob/master/doc/anti-patterns/long-method.md)
## Rules
### Common
Expand Down
13 changes: 13 additions & 0 deletions doc/anti-patterns/long-method.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Long Method

Long methods are difficult to reuse and understand because they are usually responsible for more than one thing. Separating those methods to several short ones with proper names helps you to reuse your code and understand it better without reading methods body.

Lines of code with clarification comments usually a sign for possible method extraction because you can name extracted method in a way that will cover comment description and then remove comment. Even comments for one line is a sign for method extraction.

* To shorten a method, just apply **Extract Method** command.
* If local variables and parameters prevent a method extraction, apply **Replace Temp with Query**, **Introduce Parameter Object** or **Preserve Whole Object** commands.
* Conditional statements or loops indicate the possibility of method extraction. Use **Decompose Conditional** command for conditional expressions and **Extract Method** for loops.

***

P.S. We use terminology from a book **Refactoring Improving the Design of Existing Code** by *Martin Fowler*
22 changes: 21 additions & 1 deletion lib/src/analyzer_plugin/analyzer_plugin.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import 'package:dart_code_metrics/src/rules_factory.dart';
import 'package:glob/glob.dart';
import 'package:source_span/source_span.dart';

import '../anti_patterns/base_pattern.dart';
import '../anti_patterns_factory.dart';
import '../scope_ast_visitor.dart';
import '../utils/metrics_analyzer_utils.dart';
import '../utils/yaml_utils.dart';
Expand All @@ -35,6 +37,8 @@ const _codeMetricsId = 'code-metrics';
const _defaultSkippedFolders = ['.dart_tool/**', 'packages/**'];

class MetricsAnalyzerPlugin extends ServerPlugin {
final Iterable<BasePattern> _checkingAntiPatterns;

Config _metricsConfig;
Iterable<Glob> _globalExclude;
Iterable<Glob> _metricsExclude;
Expand All @@ -54,7 +58,8 @@ class MetricsAnalyzerPlugin extends ServerPlugin {
String get version => '1.9.0';

MetricsAnalyzerPlugin(ResourceProvider provider)
: _checkingCodeRules = [],
: _checkingAntiPatterns = allPatterns,
_checkingCodeRules = [],
super(provider);

@override
Expand Down Expand Up @@ -189,6 +194,11 @@ class MetricsAnalyzerPlugin extends ServerPlugin {
.map((issue) =>
codeIssueToAnalysisErrorFixes(issue, analysisResult))));

if (_metricsConfig != null &&
!isExcluded(analysisResult, _metricsExclude)) {
result.addAll(_checkOnAntiPatterns(ignores, analysisResult, sourceUri));
}

if (_metricsConfig != null &&
!ignores.ignoreRule(_codeMetricsId) &&
!isExcluded(analysisResult, _metricsExclude)) {
Expand Down Expand Up @@ -255,6 +265,16 @@ class MetricsAnalyzerPlugin extends ServerPlugin {
return result;
}

Iterable<plugin.AnalysisErrorFixes> _checkOnAntiPatterns(IgnoreInfo ignores,
ResolvedUnitResult analysisResult, Uri sourceUri) =>
_checkingAntiPatterns
.where((pattern) => !ignores.ignoreRule(pattern.id))
.expand((pattern) => pattern.check(analysisResult.unit, sourceUri,
analysisResult.content, _metricsConfig))
.where((issue) =>
!ignores.ignoredAt(issue.patternId, issue.sourceSpan.start.line))
.map(designIssueToAnalysisErrorFixes);

AnalysisOptions _readOptions(AnalysisDriver driver) {
if (driver?.contextRoot?.optionsFilePath?.isNotEmpty ?? false) {
final file = resourceProvider.getFile(driver.contextRoot.optionsFilePath);
Expand Down
18 changes: 18 additions & 0 deletions lib/src/analyzer_plugin/analyzer_plugin_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;
import 'package:source_span/source_span.dart';

import '../models/design_issue.dart';

bool isSupported(AnalysisResult result) =>
result.path != null &&
result.path.endsWith('.dart') &&
Expand Down Expand Up @@ -50,6 +52,22 @@ plugin.AnalysisErrorFixes codeIssueToAnalysisErrorFixes(
])),
]);

plugin.AnalysisErrorFixes designIssueToAnalysisErrorFixes(DesignIssue issue) =>
plugin.AnalysisErrorFixes(plugin.AnalysisError(
plugin.AnalysisErrorSeverity.INFO,
plugin.AnalysisErrorType.HINT,
plugin.Location(
issue.sourceSpan.sourceUrl.path,
issue.sourceSpan.start.offset,
issue.sourceSpan.length,
issue.sourceSpan.start.line,
issue.sourceSpan.start.column),
issue.message,
issue.patternId,
correction: issue.recommendation,
url: issue.patternDocumentation?.toString(),
hasFix: false));

plugin.AnalysisErrorFixes metricReportToAnalysisErrorFixes(
SourceLocation startLocation,
int length,
Expand Down
18 changes: 18 additions & 0 deletions lib/src/anti_patterns/base_pattern.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:meta/meta.dart';

import '../models/config.dart';
import '../models/design_issue.dart';

abstract class BasePattern {
final String id;
final Uri documentation;

const BasePattern({
@required this.id,
@required this.documentation,
});

Iterable<DesignIssue> check(
CompilationUnit unit, Uri sourceUrl, String sourceContent, Config config);
}
66 changes: 66 additions & 0 deletions lib/src/anti_patterns/long_method.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:meta/meta.dart';
import 'package:source_span/source_span.dart';

import '../lines_of_code/lines_with_code_ast_visitor.dart';
import '../models/config.dart';
import '../models/design_issue.dart';
import '../scope_ast_visitor.dart';
import 'base_pattern.dart';

class LongMethod extends BasePattern {
static const String patternId = 'long-method';
static const _documentationUrl = 'https://git.io/JUIP7';

LongMethod()
: super(id: patternId, documentation: Uri.parse(_documentationUrl));

@override
Iterable<DesignIssue> check(CompilationUnit unit, Uri sourceUrl,
String sourceContent, Config config) {
final issues = <DesignIssue>[];

final visitor = ScopeAstVisitor();
unit.visitChildren(visitor);

for (final function in visitor.functions) {
final linesWithCodeAstVisitor = LinesWithCodeAstVisitor(unit.lineInfo);
function.declaration.visitChildren(linesWithCodeAstVisitor);

if (linesWithCodeAstVisitor.linesWithCode.length >
config.linesOfExecutableCodeWarningLevel) {
final offsetLocation =
unit.lineInfo.getLocation(function.declaration.offset);
final endLocation = unit.lineInfo.getLocation(function.declaration.end);

issues.add(DesignIssue(
patternId: id,
patternDocumentation: documentation,
sourceSpan: SourceSpanBase(
SourceLocation(function.declaration.offset,
sourceUrl: sourceUrl,
line: offsetLocation.lineNumber,
column: offsetLocation.columnNumber),
SourceLocation(function.declaration.end,
sourceUrl: sourceUrl,
line: endLocation.lineNumber,
column: endLocation.columnNumber),
sourceContent.substring(
function.declaration.offset, function.declaration.end)),
message: _compileMessage(
lines: linesWithCodeAstVisitor.linesWithCode.length),
recommendation: _compileRecomendationMessage(
maximumLines: config.linesOfExecutableCodeWarningLevel),
));
}
}

return issues;
}

String _compileMessage({@required int lines}) =>
'Long Method. This method contains $lines lines with executable code.';

String _compileRecomendationMessage({@required int maximumLines}) =>
"Based on configuration of this package, we don't recommend write a method longer than $maximumLines lines with executable code.";
}
14 changes: 14 additions & 0 deletions lib/src/anti_patterns_factory.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import 'anti_patterns/base_pattern.dart';
import 'anti_patterns/long_method.dart';

final _implementedPatterns = <String, BasePattern Function()>{
LongMethod.patternId: () => LongMethod(),
};

Iterable<BasePattern> get allPatterns =>
_implementedPatterns.keys.map((id) => _implementedPatterns[id]());

Iterable<BasePattern> getPatternsById(Iterable<String> patternsId) =>
List.unmodifiable(_implementedPatterns.keys
.where((id) => patternsId.contains(id))
.map<BasePattern>((id) => _implementedPatterns[id]()));
30 changes: 21 additions & 9 deletions lib/src/metrics_analysis_recorder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:dart_code_metrics/src/models/function_record.dart';
import 'package:path/path.dart' as p;

import 'models/component_record.dart';
import 'models/design_issue.dart';
import 'models/scoped_component_declaration.dart';
import 'models/scoped_function_declaration.dart';
import 'utils/metrics_analyzer_utils.dart';
Expand All @@ -19,6 +20,7 @@ class MetricsAnalysisRecorder
Map<ScopedComponentDeclaration, ComponentRecord> _componentRecords;
Map<ScopedFunctionDeclaration, FunctionRecord> _functionRecords;
List<CodeIssue> _issues;
List<DesignIssue> _designIssues;

final _records = <FileRecord>[];
@override
Expand Down Expand Up @@ -78,6 +80,13 @@ class MetricsAnalysisRecorder
_functionRecords[declaration] = record;
}

@override
void recordDesignIssues(Iterable<DesignIssue> issues) {
_checkState();

_designIssues.addAll(issues);
}

@override
@Deprecated('Use MetricsRecordsBuilder.recordIssues')
void recordIssues(Iterable<CodeIssue> issues) {
Expand Down Expand Up @@ -109,22 +118,25 @@ class MetricsAnalysisRecorder
_componentRecords = {};
_functionRecords = {};
_issues = [];
_designIssues = [];
}

void _endRecordFile() {
_records.add(FileRecord(
fullPath: _fileGroupPath,
relativePath: _relativeGroupPath,
components: Map.unmodifiable(
_componentRecords.map<String, ComponentRecord>((key, value) =>
MapEntry(getComponentHumanReadableName(key), value))),
functions: Map.unmodifiable(
_functionRecords.map<String, FunctionRecord>((key, value) =>
MapEntry(getFunctionHumanReadableName(key), value))),
issues: _issues));
fullPath: _fileGroupPath,
relativePath: _relativeGroupPath,
components: Map.unmodifiable(
_componentRecords.map<String, ComponentRecord>((key, value) =>
MapEntry(getComponentHumanReadableName(key), value))),
functions: Map.unmodifiable(_functionRecords.map<String, FunctionRecord>(
(key, value) => MapEntry(getFunctionHumanReadableName(key), value))),
issues: _issues,
designIssue: _designIssues,
));
_relativeGroupPath = null;
_fileGroupPath = null;
_functionRecords = null;
_issues = null;
_designIssues = null;
}
}
36 changes: 30 additions & 6 deletions lib/src/metrics_analyzer.dart
Original file line number Diff line number Diff line change
@@ -1,27 +1,34 @@
import 'package:analyzer/dart/analysis/features.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/analysis/utilities.dart';
import 'package:dart_code_metrics/src/halstead_volume/halstead_volume_ast_visitor.dart';
import 'package:dart_code_metrics/src/ignore_info.dart';
import 'package:dart_code_metrics/src/metrics_analysis_recorder.dart';
import 'package:dart_code_metrics/src/models/design_issue.dart';
import 'package:dart_code_metrics/src/models/function_record.dart';
import 'package:dart_code_metrics/src/rules/base_rule.dart';
import 'package:dart_code_metrics/src/scope_ast_visitor.dart';
import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;

import 'analysis_options.dart';
import 'anti_patterns/base_pattern.dart';
import 'anti_patterns_factory.dart';
import 'lines_of_code/lines_with_code_ast_visitor.dart';
import 'metrics/cyclomatic_complexity/control_flow_ast_visitor.dart';
import 'metrics/cyclomatic_complexity/cyclomatic_config.dart';
import 'models/component_record.dart';
import 'models/config.dart';
import 'rules_factory.dart';
import 'utils/metrics_analyzer_utils.dart';

/// Performs code quality analysis on specified files
/// See [MetricsAnalysisRunner] to get analysis info
class MetricsAnalyzer {
final Iterable<BaseRule> _checkingCodeRules;
final Iterable<BasePattern> _checkingAntiPatterns;
final Iterable<Glob> _globalExclude;
final Config _metricsConfig;
final Iterable<Glob> _metricsExclude;
final MetricsAnalysisRecorder _recorder;

Expand All @@ -30,7 +37,9 @@ class MetricsAnalyzer {
AnalysisOptions options,
}) : _checkingCodeRules =
options?.rules != null ? getRulesById(options.rules) : [],
_checkingAntiPatterns = allPatterns,
_globalExclude = _prepareExcludes(options?.excludePatterns),
_metricsConfig = options.metricsConfig,
_metricsExclude = _prepareExcludes(options?.metricsExcludePatterns);

void runAnalysis(String filePath, String rootFolder) {
Expand Down Expand Up @@ -102,14 +111,29 @@ class MetricsAnalyzer {
final ignores =
IgnoreInfo.calculateIgnores(parseResult.content, lineInfo);

builder.recordIssues(_checkingCodeRules
.where((rule) => !ignores.ignoreRule(rule.id))
.expand((rule) => rule
.check(parseResult.unit, Uri.parse(filePath), parseResult.content)
.where((issue) => !ignores.ignoredAt(
issue.ruleId, issue.sourceSpan.start.line))));
final filePathUri = Uri.parse(filePath);

builder
..recordIssues(_checkingCodeRules
.where((rule) => !ignores.ignoreRule(rule.id))
.expand((rule) => rule
.check(parseResult.unit, filePathUri, parseResult.content)
.where((issue) => !ignores.ignoredAt(
issue.ruleId, issue.sourceSpan.start.line))))
..recordDesignIssues(
_checkOnAntiPatterns(ignores, parseResult, filePathUri));
});
}

Iterable<DesignIssue> _checkOnAntiPatterns(IgnoreInfo ignores,
ParseStringResult analysisResult, Uri sourceUri) =>
_checkingAntiPatterns
.where((pattern) => !ignores.ignoreRule(pattern.id))
.expand((pattern) => pattern
.check(analysisResult.unit, sourceUri, analysisResult.content,
_metricsConfig)
.where((issue) => !ignores.ignoredAt(
issue.patternId, issue.sourceSpan.start.line)));
}

Iterable<Glob> _prepareExcludes(Iterable<String> patterns) =>
Expand Down
3 changes: 3 additions & 0 deletions lib/src/metrics_records_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import 'package:dart_code_metrics/src/models/function_record.dart';
import 'package:dart_code_metrics/src/models/scoped_component_declaration.dart';
import 'package:dart_code_metrics/src/models/scoped_function_declaration.dart';

import 'models/design_issue.dart';

abstract class MetricsRecordsBuilder {
void recordComponent(
ScopedComponentDeclaration declaration, ComponentRecord record);
void recordFunction(
ScopedFunctionDeclaration declaration, FunctionRecord record);
void recordDesignIssues(Iterable<DesignIssue> issues);
void recordIssues(Iterable<CodeIssue> issues);
}
19 changes: 19 additions & 0 deletions lib/src/models/design_issue.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import 'package:meta/meta.dart';
import 'package:source_span/source_span.dart';

@immutable
class DesignIssue {
final String patternId;
final Uri patternDocumentation;
final SourceSpanBase sourceSpan;
final String message;
final String recommendation;

const DesignIssue({
@required this.patternId,
@required this.patternDocumentation,
@required this.sourceSpan,
@required this.message,
this.recommendation,
});
}
Loading

0 comments on commit 4e66521

Please sign in to comment.