Skip to content
This repository was archived by the owner on Nov 20, 2024. 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
8 changes: 4 additions & 4 deletions lib/src/meta.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ library linter_meta;
/// a given list of rules `r1`...`rn`.
///
/// For example, the incompatibility between `prefer_local_finals` and
/// `unnecessary_finals` could be captured in the `PreferFinalLocals` class
/// `unnecessary_final` could be captured in the `PreferFinalLocals` class
/// declaration like this:
///
/// @IncompatibleWith(['unnecessary_finals'])
/// @IncompatibleWith(['unnecessary_final'])
/// class PreferFinalLocals extends LintRule implements NodeLintRule {
/// ...
/// }
///
/// For consistency of documentation, incompatibility should be declared in both
/// directions. That is, all conflicting rules `r1`...`rn` should annotate
/// their incompatibility with `r`. In this case, `'unnecessary_finals'` would
/// their incompatibility with `r`. In this case, `'unnecessary_final'` would
/// look like this:
///
/// @IncompatibleWith(['prefer_local_finals'])
/// class UnnecessaryFinals extends LintRule implements NodeLintRule {
/// class UnnecessaryFinal extends LintRule implements NodeLintRule {
/// ...
/// }
///
Expand Down
2 changes: 2 additions & 0 deletions lib/src/rules/prefer_final_locals.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';

import '../analyzer.dart';
import '../meta.dart';

const _desc =
r'Prefer final for variable declarations if they are not reassigned.';
Expand Down Expand Up @@ -46,6 +47,7 @@ void mutableCase() {

''';

@IncompatibleWith(['unnecessary_final'])
class PreferFinalLocals extends LintRule implements NodeLintRule {
PreferFinalLocals()
: super(
Expand Down
2 changes: 2 additions & 0 deletions lib/src/rules/unnecessary_final.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';

import '../analyzer.dart';
import '../meta.dart';

const _desc = "Don't use `final` for local variables.";

Expand Down Expand Up @@ -35,6 +36,7 @@ void goodMethod() {
```
''';

@IncompatibleWith(['prefer_final_locals'])
class UnnecessaryFinal extends LintRule implements NodeLintRule {
UnnecessaryFinal()
: super(
Expand Down
124 changes: 124 additions & 0 deletions lib/src/util/lint_cache.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:io';

import 'package:analyzer/dart/analysis/analysis_context_collection.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/file_system/physical_file_system.dart';

import '../analyzer.dart';
import '../rules.dart';
import '../utils.dart';

const String _INCOMPATIBLE_WITH_CLASS_NAME = 'IncompatibleWith';
const String _LINT_RULE_CLASS_NAME = 'LintRule';
const String _LINTER_META_LIB_NAME = 'linter_meta';

bool _isIncompatibleWithAnnotation(Element element) =>
element is ConstructorElement &&
element.enclosingElement.name == _INCOMPATIBLE_WITH_CLASS_NAME &&
element.library?.name == _LINTER_META_LIB_NAME;

class LintCache {
List<LintRuleDetails> details = <LintRuleDetails>[];

bool _initialized = false;

LintRuleDetails findDetailsByClassName(String className) {
for (var detail in details) {
if (detail.className == className) {
return detail;
}
}
return null;
}

LintRuleDetails findDetailsById(String id) {
for (var detail in details) {
if (detail.id == id) {
return detail;
}
}
return null;
}

Future<void> init() async {
if (_initialized) {
return;
}
registerLintRules();

// Setup details.
for (var lint in Analyzer.facade.registeredRules) {
details.add(LintRuleDetails(lint.runtimeType.toString(), lint.name));
}

// Process compatibility annotations.
final rulePath = File('lib/src/rules').absolute.path;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we could analyze and visit the constructor invocations in lib/src/rules.dart. That would probably find all of the lint rule class element faster (although it would miss classes that aren't registered).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss refining this together. I have a few other ideas but a whiteboard would make them easier.

I'll land this as a jumping off point.

final collection = AnalysisContextCollection(
includedPaths: [rulePath],
resourceProvider: PhysicalResourceProvider.INSTANCE,
);

final visitor = _LintVisitor();
for (var context in collection.contexts) {
for (var filePath in context.contextRoot.analyzedFiles()) {
if (isDartFileName(filePath)) {
final result = await context.currentSession.getResolvedUnit(filePath);
result.unit.accept(visitor);
}
}
}

for (var classElement in visitor.classElements) {
for (var annotation in classElement.metadata) {
if (_isIncompatibleWithAnnotation(annotation.element)) {
final constantValue = annotation.computeConstantValue();
final ruleObjects = constantValue?.getField('rules')?.toListValue();
if (ruleObjects != null) {
final lintClassName = classElement.thisType.name;
final ruleDetails = findDetailsByClassName(lintClassName);
for (var ruleObject in ruleObjects) {
final ruleId = ruleObject.toStringValue();
ruleDetails.incompatibleRules.add(ruleId);
}
}
}
}
}
_initialized = true;
}
}

class LintRuleDetails {
final String className;
final String id;
final List<String> incompatibleRules = <String>[];
LintRuleDetails(this.className, this.id);

@override
String toString() => '$className:$id';
}

class _LintVisitor extends RecursiveAstVisitor {
final List<ClassElement> classElements = <ClassElement>[];

@override
void visitClassDeclaration(ClassDeclaration node) {
final classElement = node.declaredElement;
if (classElement == null) {
return;
}

for (var superType in classElement.allSupertypes) {
if (superType.name == _LINT_RULE_CLASS_NAME) {
classElements.add(classElement);
return;
}
}
}
}
2 changes: 2 additions & 0 deletions test/all.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import 'rule_test.dart' as rule_test;
import 'utils_test.dart' as utils_test;
import 'validate_format_test.dart' as validate_format;
import 'validate_headers_test.dart' as validate_headers;
import 'validate_metadata.dart' as validate_metadata;
import 'version_test.dart' as version_test;

void main() {
Expand All @@ -27,5 +28,6 @@ void main() {
utils_test.main();
validate_format.main();
validate_headers.main();
validate_metadata.main();
version_test.main();
}
31 changes: 31 additions & 0 deletions test/validate_metadata.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:linter/src/analyzer.dart';
import 'package:linter/src/rules.dart';
import 'package:linter/src/util/lint_cache.dart';
import 'package:test/test.dart';

void main() {
final lintCache = LintCache();

group('check for incompatible rules:', () {
registerLintRules();
for (var rule in Analyzer.facade.registeredRules) {
test(rule.name, () async {
await lintCache.init();
var lintDetail = lintCache.findDetailsById(rule.name);
for (var incompatibleRule in lintDetail.incompatibleRules) {
final ruleDetail = lintCache.findDetailsById(incompatibleRule);
expect(ruleDetail, isNotNull,
reason:
'No rule found for id: $incompatibleRule (check for typo?)');
expect(ruleDetail.incompatibleRules, contains(lintDetail.id),
reason:
'${ruleDetail.id} should declare ${lintDetail.id} as `@IncompatibleWith` but does not.');
}
});
}
});
}