From 62f01f07fe4a44d1d0e9bd9cb8993408cf7b32d8 Mon Sep 17 00:00:00 2001 From: Christophe Zurn Date: Tue, 2 May 2023 17:52:15 +0200 Subject: [PATCH] Update documentation, disallow standard rule format, add allowed sections in 'Why is it an issue?' --- docs/description.adoc | 50 ++++--------------- ..._names.adoc => hotspot_section_names.adoc} | 11 ++-- .../rspec_tools/validation/description.py | 10 ++-- rspec-tools/tests/test_cli.py | 2 +- .../validation/test_description_validation.py | 8 +-- 5 files changed, 25 insertions(+), 56 deletions(-) rename docs/header_names/{legacy_section_names.adoc => hotspot_section_names.adoc} (70%) diff --git a/docs/description.adoc b/docs/description.adoc index 9e7e2a32fb..14494e9972 100644 --- a/docs/description.adoc +++ b/docs/description.adoc @@ -8,44 +8,13 @@ See also the <> and https://docs.sonarqu There should be no first level titles (`+= Title+`) in your adoc. -The following are the only allowed second level titles for H2 for standard and hotspot rules -(for the education format, see the detailed <<3. Education Format,Education format>> section to see which headers are allowed): - -include::./header_names/legacy_section_names.adoc[] - -Most third-level and fourth-level titles (`+=== Title t3+` and `+==== Title t4+`) are not checked, the only type of rule description for which some of these -titles are checked are the education format rules (see below). +The allowed second level titles and lower are described in their respective sections below, for each type of rule description. == Types of rule description -The above list of allowed H2 sections is an absolute list and covers all the different types of rule descriptions that exist. -However, depending on the type of rule description, the expected order and allowed sections are not the same. -There are currently 3 types of rule descriptions, each having a specific structure. +There are currently 2 types of rule descriptions, each having a specific structure. -=== 1. Standard - -The standard rule description is the currently most widespread format. It includes all _Code Smells_, _Bugs_ and most _Vulnerabilities_. - -This is the "classical" format that starts with a description of the rule, and is usually followed by code examples of bad and good practices, -with some links to resources at the end. - -This format has the structure of the following section: - -. Description (no title) -. Noncompliant Code Example -. Compliant Solution -. Exceptions -. See -. See Also - -Note that most sections are optional, only the description is mandatory. - -=== 2. Hotspot - -Specific to the _Hotspot_ issue type, this format is mostly similar to the standard one, -with the addition of 2 new sections: `Ask Yourself Whether` and `Recommended Secure Coding Practices`. Additionally, the `Noncompliant Code Example` -section has been renamed into `Sensitive Code Example`: this is based on the fact that for _Hotspots_ there is no certainty that what was raised -is an actual vulnerability. +=== 1. Hotspot The possible sections for this format are the following: @@ -58,19 +27,20 @@ The possible sections for this format are the following: . See . See Also -=== 3. Education Format +Third-level and fourth-level titles (`+=== Title t3+` and `+==== Title t4+`) are not checked for this type of rule. -Finally, the last format is the one related to the Progressive Education. -This new format has a completely different structure, with additional subsections and information, -to be as comprehensible as possible for the users. +=== 2. Education Format -This new structure is defined as follows: +This format is defined as follows: * Short description (no title) // This needs to be kept in sync with the [maps in the validation script](https://github.com/SonarSource/rspec/blob/master/rspec-tools/rspec_tools/validation/description.py#L32-L39). * Why is this an issue? -** What is the potential impact? +** What is the potential impact? (optional) +** Noncompliant code example (optional) +** Compliant solution (optional) +** Exceptions (optional) * How to fix it (optional) ** Code examples *** Noncompliant code example diff --git a/docs/header_names/legacy_section_names.adoc b/docs/header_names/hotspot_section_names.adoc similarity index 70% rename from docs/header_names/legacy_section_names.adoc rename to docs/header_names/hotspot_section_names.adoc index 50296c0941..5661ee319f 100644 --- a/docs/header_names/legacy_section_names.adoc +++ b/docs/header_names/hotspot_section_names.adoc @@ -1,9 +1,6 @@ -* Noncompliant Code Example -* Compliant Solution -* See -* See Also -* Exceptions -* Sensitive Code Example * Ask Yourself Whether +* Sensitive Code Example * Recommended Secure Coding Practices -* Deprecated +* Compliant Solution +* Exceptions +* See diff --git a/rspec-tools/rspec_tools/validation/description.py b/rspec-tools/rspec_tools/validation/description.py index baf2f471de..5cc1eaf939 100644 --- a/rspec-tools/rspec_tools/validation/description.py +++ b/rspec-tools/rspec_tools/validation/description.py @@ -24,13 +24,13 @@ HOW_TO_FIX_IT_REGEX = re.compile(HOW_TO_FIX_IT) # in the migrated RSPECs. # Further work required to shorten the list by renaming the sections in some RSPECS # to keep only on version for each title. -LEGACY_SECTION_NAMES: Final[list[str]] = parse_names('docs/header_names/legacy_section_names.adoc') +HOTSPOT_SECTION_NAMES: Final[list[str]] = parse_names('docs/header_names/hotspot_section_names.adoc') # The list of all the framework names currently accepted by the script. ACCEPTED_FRAMEWORK_NAMES: Final[list[str]] = parse_names('docs/header_names/allowed_framework_names.adoc') -# This needs to be kept in sync with the [headers list in docs/descriptions.adoc](https://github.com/SonarSource/rspec/blob/master/docs/description.adoc#3-progressive-education) +# This needs to be kept in sync with the [headers list in docs/descriptions.adoc](https://github.com/SonarSource/rspec/blob/master/docs/description.adoc#2-education-format) SECTIONS = { - 'Why is this an issue?': ['What is the potential impact?'] + 'Why is this an issue?': ['What is the potential impact?', 'Noncompliant code example', 'Compliant solution', 'Exceptions'] } OPTIONAL_SECTIONS = { # Also covers 'How to fix it in {Framework Display Name}' @@ -61,9 +61,9 @@ def validate_section_names(rule_language: LanguageSpecificRule): # All mandatory titles have to be present in the rule description. raise RuleValidationError(f'Rule {rule_language.id} is missing the "{missing_titles[0]}" section') else: - # we're using the legacy format + # Using the hotspot format. for title in h2_titles: - if title not in LEGACY_SECTION_NAMES: + if title not in HOTSPOT_SECTION_NAMES: raise RuleValidationError(f'Rule {rule_language.id} has an unconventional header "{title}"') def validate_how_to_fix_it_sections_names(rule_language: LanguageSpecificRule, h2_titles: list[str]): diff --git a/rspec-tools/tests/test_cli.py b/rspec-tools/tests/test_cli.py index 0d43ef8fd0..b8a7eaa256 100644 --- a/rspec-tools/tests/test_cli.py +++ b/rspec-tools/tests/test_cli.py @@ -91,7 +91,7 @@ class TestCLIValidateDescription: return runner.invoke(cli, arguments) def test_valid_rule(self): - result = self._run(['S100']) + result = self._run(['S3649']) assert result.output == '' assert result.exit_code == 0 diff --git a/rspec-tools/tests/validation/test_description_validation.py b/rspec-tools/tests/validation/test_description_validation.py index d5d21b8ac5..edd9f00ba4 100644 --- a/rspec-tools/tests/validation/test_description_validation.py +++ b/rspec-tools/tests/validation/test_description_validation.py @@ -22,9 +22,11 @@ def invalid_rule(mockinvalidrules: Path): return rule.get_language(language) return _invalid_rule -def test_valid_sections_passes_validation(rule_language): - '''Check that description with standard sections is considered valid.''' - validate_section_names(rule_language('S100', 'cfamily')) +def test_legacy_sections_fails_validation(rule_language): + '''Check that description with standard sections are no longer considered valid.''' + rule = rule_language('S100', 'cfamily') + with pytest.raises(RuleValidationError, match=fr'^Rule {rule.id} has an unconventional header "Noncompliant Code Example"'): + validate_section_names(rule) def test_unexpected_section_fails_validation(invalid_rule): rule = invalid_rule('S100', 'cfamily')