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

Commit

Permalink
Merge 5d855ef into 28eea04
Browse files Browse the repository at this point in the history
  • Loading branch information
dkrutskikh committed Aug 29, 2020
2 parents 28eea04 + 5d855ef commit d0f7f80
Show file tree
Hide file tree
Showing 19 changed files with 447 additions and 103 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

Code based on short methods live the longest and more useful for reuse. The longer method, the more difficult it is to understand it. If the method has a good name, then you don't need to look at its body.

If you feel need to comment on something in code, you need extract this block to a method. It makes sense to allocate even one line into a method if it needs clarification.

* To shorten the method, just apply the **Extract Method**.
* If local variables and parameters prevent to method allocation, you can apply **Replace Temp with Query**, **Introduce Parameter Object** or **Preserve Whole Object**.
* Conditional statements or loops indicate the possibility of extract into a separate method. For working with a conditional expressions **Decompose Conditional** is suitable. To work with a loops use **Extract Method**.

***

P.S. We use terminology from 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 d0f7f80

Please sign in to comment.