Skip to content

Commit

Permalink
feat(tool): include tooling to track excluded lint rules and their re…
Browse files Browse the repository at this point in the history
…asons (#100)

* feat: make tooling to generate tables with excluded rules

* feat: update docs, comments and names

* docs: add emojis

* feat: include links

* refactor: use "excluded" over "disabled"

* chore: formatted exclusion_reasons.json

* chore: update pubspec.yaml

* docs: update README phrasing

* docs: updated linter_rules.dart library doc

* ci: add workflow

* ci: add analyze_directories

* ci: update workflow

* ci: add path to format command

* docs: remove comma

* chore: update reason for prefer_double_quotes

* docs: update README.md

* docs: add reason for prefer relative imports

* docs: update excluded section

* docs: discarded_futures reason

* docs: improved docs

* docs: add missing comma

* docs: clarified doc

* docs: missing comma

* docs: add reason for use_decorated_box

* feat: automatic replacement of table

* docs: update CONTRIBUTING.md

* docs: update exclusion reason table documentation

* feat: remove old exclusions

* docs: add on

* docs: update docs

* docs: add more docs

* docs: typo in "star"

* refactor: remove readme variable

* docs: annotate_redeclares as Experimental

* docs: include reason for always_put_control_body_on_new_line

* docs: add reason for always_specify_types

* feat: use JSON over scrape
  • Loading branch information
alestiago authored Jun 11, 2024
1 parent 64b3999 commit aea906c
Show file tree
Hide file tree
Showing 16 changed files with 448 additions and 3 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ jobs:
run: dart pub get

- name: Format
run: dart format --set-exit-if-changed .
run: dart format --set-exit-if-changed lib example

- name: Analyze
run: dart analyze .
run: dart analyze lib example

pana:
uses: VeryGoodOpenSource/very_good_workflows/.github/workflows/pana.yml@v1
29 changes: 29 additions & 0 deletions .github/workflows/tool_linter_rules.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
name: linter_rules (tool)

on: pull_request

jobs:
build:
defaults:
run:
working-directory: tool/linter_rules

runs-on: ubuntu-latest

steps:
- name: 📚 Git Checkout
uses: actions/checkout@v4

- name: 🎯 Setup Dart
uses: dart-lang/setup-dart@v1
with:
sdk: 3.4.0

- name: 📦 Install Dependencies
run: dart pub get

- name: ✨ Check Formatting
run: dart format --line-length 80 --set-exit-if-changed .

- name: 🕵️ Analyze
run: dart analyze --fatal-infos --fatal-warnings
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,5 @@ To release a new version:

1. Copy the most recent yaml to a new one with the new desired version.
1. Include that file on the main yaml file `lib/analysis_options.yaml`.
1. Update the `README.md` exclusion table, refer to the ["Exclusion Reason Table 🗞️👨‍⚖️"](tool/linter_rules/README.md#exclusion-reason-table-️️) documentation for more information.
1. Open a pull request with the proposed changes.

42 changes: 42 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,48 @@ To indicate your project is using `very_good_analysis` →
[![style: very good analysis](https://img.shields.io/badge/style-very_good_analysis-B22C89.svg)](https://pub.dev/packages/very_good_analysis)
```

## Excluded rules

Below is a list of rules that are not enabled by default together with the reason on why they have been excluded:

<!-- start:excluded_rules_table -->

| Rule | Reason |
| ---------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- |
| [`always_put_control_body_on_new_line`](https://dart.dev/tools/linter-rules/always_put_control_body_on_new_line) | [Can conflict with the Dart formatter](https://dart.dev/tools/linter-rules/always_put_control_body_on_new_line) |
| [`always_specify_types`](https://dart.dev/tools/linter-rules/always_specify_types) | Incompatible with [omit_local_variable_types](https://dart.dev/tools/linter-rules/omit_local_variable_types) |
| [`annotate_redeclares`](https://dart.dev/tools/linter-rules/annotate_redeclares) | Experimental |
| [`avoid_annotating_with_dynamic`](https://dart.dev/tools/linter-rules/avoid_annotating_with_dynamic) | Not specified |
| [`avoid_catches_without_on_clauses`](https://dart.dev/tools/linter-rules/avoid_catches_without_on_clauses) | Not specified |
| [`avoid_classes_with_only_static_members`](https://dart.dev/tools/linter-rules/avoid_classes_with_only_static_members) | Not specified |
| [`avoid_implementing_value_types`](https://dart.dev/tools/linter-rules/avoid_implementing_value_types) | Not specified |
| [`avoid_types_on_closure_parameters`](https://dart.dev/tools/linter-rules/avoid_types_on_closure_parameters) | Not specified |
| [`close_sinks`](https://dart.dev/tools/linter-rules/close_sinks) | Not specified |
| [`deprecated_member_use_from_same_package`](https://dart.dev/tools/linter-rules/deprecated_member_use_from_same_package) | Not specified |
| [`diagnostic_describe_all_properties`](https://dart.dev/tools/linter-rules/diagnostic_describe_all_properties) | Not specified |
| [`discarded_futures`](https://dart.dev/tools/linter-rules/discarded_futures) | [Has unresolved false positives](https://github.com/VeryGoodOpenSource/very_good_analysis/issues/74#issuecomment-1668425410) |
| [`do_not_use_environment`](https://dart.dev/tools/linter-rules/do_not_use_environment) | Not specified |
| [`matching_super_parameters`](https://dart.dev/tools/linter-rules/matching_super_parameters) | Not specified |
| [`missing_code_block_language_in_doc_comment`](https://dart.dev/tools/linter-rules/missing_code_block_language_in_doc_comment) | Not specified |
| [`no_literal_bool_comparisons`](https://dart.dev/tools/linter-rules/no_literal_bool_comparisons) | Not specified |
| [`no_self_assignments`](https://dart.dev/tools/linter-rules/no_self_assignments) | Not specified |
| [`no_wildcard_variable_uses`](https://dart.dev/tools/linter-rules/no_wildcard_variable_uses) | Not specified |
| [`prefer_double_quotes`](https://dart.dev/tools/linter-rules/prefer_double_quotes) | Incompatible with [prefer_single_quotes](https://dart.dev/tools/linter-rules/prefer_single_quotes) |
| [`prefer_expression_function_bodies`](https://dart.dev/tools/linter-rules/prefer_expression_function_bodies) | Not specified |
| [`prefer_final_parameters`](https://dart.dev/tools/linter-rules/prefer_final_parameters) | Not specified |
| [`prefer_foreach`](https://dart.dev/tools/linter-rules/prefer_foreach) | Not specified |
| [`prefer_mixin`](https://dart.dev/tools/linter-rules/prefer_mixin) | Not specified |
| [`prefer_relative_imports`](https://dart.dev/tools/linter-rules/prefer_relative_imports) | Incompatible with [always_use_package_imports](https://dart.dev/tools/linter-rules/always_use_package_imports) |
| [`type_literal_in_constant_pattern`](https://dart.dev/tools/linter-rules/type_literal_in_constant_pattern) | Not specified |
| [`unnecessary_final`](https://dart.dev/tools/linter-rules/unnecessary_final) | Not specified |
| [`unnecessary_library_name`](https://dart.dev/tools/linter-rules/unnecessary_library_name) | Not specified |
| [`unnecessary_null_aware_operator_on_extension_on_nullable`](https://dart.dev/tools/linter-rules/unnecessary_null_aware_operator_on_extension_on_nullable) | Not specified |
| [`unreachable_from_main`](https://dart.dev/tools/linter-rules/unreachable_from_main) | Not specified |
| [`unsafe_html`](https://dart.dev/tools/linter-rules/unsafe_html) | Not specified |
| [`use_decorated_box`](https://dart.dev/tools/linter-rules/use_decorated_box) | [Has unresolved malfunctions](https://github.com/VeryGoodOpenSource/very_good_analysis/issues/65) |

<!-- end:excluded_rules_table -->

[analysis_options_yaml]: https://github.com/VeryGoodOpenSource/very_good_analysis/blob/main/lib/analysis_options.5.1.0.yaml
[ci_badge]: https://github.com/VeryGoodOpenSource/very_good_analysis/workflows/ci/badge.svg
[ci_badge_link]: https://github.com/VeryGoodOpenSource/very_good_analysis/actions
Expand Down
7 changes: 7 additions & 0 deletions tool/linter_rules/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# See https://www.dartlang.org/guides/libraries/private-files

# Files and directories created by pub
.dart_tool/
.packages
build/
pubspec.lock
27 changes: 27 additions & 0 deletions tool/linter_rules/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Linter Rules Tool ⚙️

Tools that help maintain Very Good Analysis rules.

## Exclusion Reason Table 🗞️👨‍⚖️

For each rule that is not enabled by default by Very Good Analysis, we create a table with the rule name and the reason on why it is not enabled by default, in the following format:

```md
| Rule Name | Reason |
| --------- | ------- |
| rule1 | Reason1 |
```

The reasons are defined in the [`exclusion_reasons.json`](exclusion_reasons.json) file, where each rule that is not enabled by default has an entry with its rule name and the reason on why it is not enabled by default.

### Usage

To generate the exclusion reason table, run the following command (from tool/linter_rules):

```sh
dart lib/exclusion_reason_table.dart $version
```

This command will update the README table for the rules that are not enabled by default in the specified `$version` of Very Good Analysis. The `$version` is a user specified argument and it should be in the format `x.y.z`. In addition, those no longer excluded rules will be removed from the `exclusion_reasons.json` file. The command does not format the output, so it is recommended to format both files, with the preferred formatter, after running the command.

Those rules that are missing a reason in the `exclusion_reasons.json` file will be added to the `exclusion_reasons.json` file with the reason `Not specified`.
1 change: 1 addition & 0 deletions tool/linter_rules/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include: package:very_good_analysis/analysis_options.yaml
33 changes: 33 additions & 0 deletions tool/linter_rules/exclusion_reasons.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
{
"always_put_control_body_on_new_line": "[Can conflict with the Dart formatter](https://dart.dev/tools/linter-rules/always_put_control_body_on_new_line)",
"always_specify_types": "Incompatible with [omit_local_variable_types](https://dart.dev/tools/linter-rules/omit_local_variable_types)",
"annotate_redeclares": "Experimental",
"avoid_annotating_with_dynamic": "Not specified",
"avoid_catches_without_on_clauses": "Not specified",
"avoid_classes_with_only_static_members": "Not specified",
"avoid_implementing_value_types": "Not specified",
"avoid_types_on_closure_parameters": "Not specified",
"close_sinks": "Not specified",
"deprecated_member_use_from_same_package": "Not specified",
"diagnostic_describe_all_properties": "Not specified",
"discarded_futures": "[Has unresolved false positives](https://github.com/VeryGoodOpenSource/very_good_analysis/issues/74#issuecomment-1668425410)",
"do_not_use_environment": "Not specified",
"matching_super_parameters": "Not specified",
"missing_code_block_language_in_doc_comment": "Not specified",
"no_literal_bool_comparisons": "Not specified",
"no_self_assignments": "Not specified",
"no_wildcard_variable_uses": "Not specified",
"prefer_double_quotes": "Incompatible with [prefer_single_quotes](https://dart.dev/tools/linter-rules/prefer_single_quotes)",
"prefer_expression_function_bodies": "Not specified",
"prefer_final_parameters": "Not specified",
"prefer_foreach": "Not specified",
"prefer_mixin": "Not specified",
"prefer_relative_imports": "Incompatible with [always_use_package_imports](https://dart.dev/tools/linter-rules/always_use_package_imports)",
"type_literal_in_constant_pattern": "Not specified",
"unnecessary_final": "Not specified",
"unnecessary_library_name": "Not specified",
"unnecessary_null_aware_operator_on_extension_on_nullable": "Not specified",
"unreachable_from_main": "Not specified",
"unsafe_html": "Not specified",
"use_decorated_box": "[Has unresolved malfunctions](https://github.com/VeryGoodOpenSource/very_good_analysis/issues/65)"
}
89 changes: 89 additions & 0 deletions tool/linter_rules/lib/exclusion_reason_table.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import 'package:linter_rules/linter_rules.dart';

/// The reason to fallback to if no reason is found in the exclusion reasons
/// file.
const _noReasonFallback = 'Not specified';

/// The tag delimiting the start and end of the excluded rules table in the
/// README.md file.
const ReadmeTag _excludedRulesTableTag = (
'<!-- start:excluded_rules_table -->',
'<!-- end:excluded_rules_table -->',
);

/// The link to the documentation for the given linter [rule].
String _linterRuleLink(String rule) {
return 'https://dart.dev/tools/linter-rules/$rule';
}

/// Updates the README table with all those rules that are not enabled by
/// Very Good Analysis in the given version, together with the reason for
/// disabling them.
///
/// If no reason is found in the exclusion reasons file, it will default to
/// [_noReasonFallback]. Those rules that are not found in the exclusion reasons
/// will then be written to the exclusion reasons file with the default reason.
/// The exclusion reasons file will not be formatted after it is updated by the
/// tool, hence, for it to be a readable JSON, a JSON formatter should be run
/// after running this tool to format it.
///
/// Those rules that were previously excluded but are now enabled by Very Good
/// Analysis will not be included in the table, and their exclusion reason will
/// be removed from the exclusion reasons file.
///
/// Should be run from the root of the `linter_rules` package (tool/linter_rules),
/// with the version of the Very Good Analysis to update the documentation for
/// as the first argument.
///
/// The version argument should be in the format of `x.y.z`. For example,
/// `5.1.0`.
///
/// To use the tool run (from tool/linter_rules):
/// ```sh
/// dart lib/exclusion_reason_table.dart $version
/// ```
///
/// Where `$version` is the version of the Very Good Analysis to log the table
/// for.
///
/// The new table will be written to the README.md file. However, it might not
/// follow the same formatting as the rest of the file, so it is recommended to
/// manually format it after running the tool.
Future<void> main(
List<String> args, {
void Function(String) log = print,
}) async {
final version = args[0];

final linterRules = (await allLinterRules()).toSet();
log('Found ${linterRules.length} available linter rules');

final veryGoodAnalysisRules =
(await allVeryGoodAnalysisRules(version: version)).toSet();
log('Found ${veryGoodAnalysisRules.length} Very Good Analysis rules');

final excludedRules = linterRules.difference(veryGoodAnalysisRules).toList()
..sort();
log('Found ${excludedRules.length} excluded rules');

final previousExclusionReasons = await readExclusionReasons();
final exclusionReasons = {
for (final rule in excludedRules)
rule: previousExclusionReasons[rule] ?? _noReasonFallback,
};
await writeExclusionReasons(exclusionReasons);

final markdownTable = generateMarkdownTable(
[
['Rule', 'Reason'],
...excludedRules.map((rule) {
final ruleMarkdownLink = '[`$rule`](${_linterRuleLink(rule)})';
return [ruleMarkdownLink, exclusionReasons[rule]!];
}),
],
);

await Readme().updateTagContent(_excludedRulesTableTag, '\n$markdownTable');

log('''Updated the README.md file with the excluded rules table.''');
}
8 changes: 8 additions & 0 deletions tool/linter_rules/lib/linter_rules.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/// Tools that help maintain Very Good Analysis rules.
library;

export 'src/all_linter_rules.dart';
export 'src/all_vga_rules.dart';
export 'src/linter_rules_reasons.dart';
export 'src/markdown_table_generator.dart';
export 'src/readme.dart';
36 changes: 36 additions & 0 deletions tool/linter_rules/lib/src/all_linter_rules.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import 'dart:convert';

import 'package:http/http.dart';

/// The [Uri] to fetch all linter rules from.
final _allLinterRulesUri = Uri.parse(
'https://raw.githubusercontent.com/dart-lang/sdk/main/pkg/linter/tool/machine/rules.json',
);

/// Fetches all linter rules names currently available in the Dart Language.
///
/// It reads and parses from a JSON file at [_allLinterRulesUri].
///
/// Those linter rules that have been removed are not included in the list.
/// In addition, those linter rules that are related to a Dart SDK that is
/// working in progress are also not included.
Future<Iterable<String>> allLinterRules() async {
final response = await get(_allLinterRulesUri);

final data = (jsonDecode(response.body) as List<dynamic>)
..removeWhere((data) {
final rule = data as Map<String, dynamic>;
final state = rule['state'] as String;
return state == 'removed';
})
..removeWhere((data) {
final rule = data as Map<String, dynamic>;
final sdk = rule['sinceDartSdk'] as String;
return sdk.contains('wip');
});

return data.map((data) {
final rule = data as Map<String, dynamic>;
return rule['name'] as String;
});
}
Loading

0 comments on commit aea906c

Please sign in to comment.