RULEAPI-785 RSPEC: education format "How to fix it" section should be optional
This commit is contained in:
parent
8e459cca14
commit
daea3fea27
@ -71,14 +71,14 @@ This new structure is defined as follows:
|
||||
// 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#L33-L40).
|
||||
* Why is this an issue?
|
||||
** What is the potential impact?
|
||||
* How to fix it
|
||||
* How to fix it (optional)
|
||||
** Code examples
|
||||
*** Noncompliant code example
|
||||
*** Compliant solution
|
||||
** How does this work?
|
||||
** Pitfalls
|
||||
** Going the extra mile
|
||||
* How to fix it in {Framework Display Name}
|
||||
* How to fix it in {Framework Display Name} (optional)
|
||||
** Code examples
|
||||
*** Noncompliant code example
|
||||
*** Compliant solution
|
||||
@ -113,7 +113,7 @@ This section can include multiple concrete threats as well.
|
||||
https://github.com/SonarSource/rspec/blob/a51217c6d91abfa5e1d77d0ae0843e3903adf2d0/rules/S3649/impact.adoc[_Example._] +
|
||||
Goal: Our users should understand the impact of this issue on their project.
|
||||
+
|
||||
* *How to fix it* or *How to fix it in `{Framework Display Name}`* (level 2 title) [the title depends on whether the description is generic or framework-specifc. See examples below.]
|
||||
* *How to fix it* or *How to fix it in `{Framework Display Name}`* (level 2 title) [Optional; the title depends on whether the description is generic or framework-specifc. See examples below.]
|
||||
+
|
||||
This section consists of one or multiple fixes for this type of issue (`Noncompliant code example` vs. `Compliant solution`). There can be multiple fixes for different libraries and/or frameworks. +
|
||||
Goal: Get an idea of how this issue can be fixed for my project/framework, why this works, what to look out for, and also how to continue improving on this topic.
|
||||
@ -145,8 +145,7 @@ Goal: Allow the user to dig deeper by providing a curated list of resources.
|
||||
* *Standards* (level 3 title) [Optional]
|
||||
* *Benchmarks* (level 3 title) [Optional]
|
||||
|
||||
Note that most sections and subsections are optional, only the `Why is this an issue?` and `How to fix it`/ `How to fix it in {Framework Display Name}` main
|
||||
sections are mandatory.
|
||||
Note that most sections and subsections are optional, only the `Why is this an issue?` main section is mandatory.
|
||||
|
||||
Content of the section "_How to fix it_ / _How to fix it in {Framework Display Name}_" can either be generic or framework specific.
|
||||
|
||||
|
@ -1,7 +1,6 @@
|
||||
from bs4 import BeautifulSoup
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
from collections import Counter
|
||||
|
||||
from rspec_tools.errors import RuleValidationError
|
||||
from rspec_tools.rules import LanguageSpecificRule
|
||||
@ -31,11 +30,11 @@ ACCEPTED_FRAMEWORK_NAMES: Final[list[str]] = parse_names('docs/header_names/allo
|
||||
|
||||
# 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)
|
||||
SECTIONS = {
|
||||
'Why is this an issue?': ['What is the potential impact?'],
|
||||
# Also covers 'How to fix it in {Framework Display Name}'
|
||||
'How to fix it': ['Code examples', 'How does this work?', 'Pitfalls', 'Going the extra mile'],
|
||||
'Why is this an issue?': ['What is the potential impact?']
|
||||
}
|
||||
OPTIONAL_SECTIONS = {
|
||||
# Also covers 'How to fix it in {Framework Display Name}'
|
||||
'How to fix it': ['Code examples', 'How does this work?', 'Pitfalls', 'Going the extra mile'],
|
||||
'Resources': ['Documentation', 'Articles & blog posts', 'Conference presentations', 'Standards', 'Benchmarks']
|
||||
}
|
||||
SUBSECTIONS = {
|
||||
@ -55,14 +54,12 @@ def validate_section_names(rule_language: LanguageSpecificRule):
|
||||
|
||||
education_titles = intersection(h2_titles, list(SECTIONS.keys()) + list(OPTIONAL_SECTIONS.keys()))
|
||||
if education_titles:
|
||||
# we're using the education format
|
||||
# Using the education format.
|
||||
validate_how_to_fix_it_sections_names(rule_language, h2_titles)
|
||||
missing_titles = difference(list(SECTIONS.keys()), education_titles)
|
||||
# we handled "how to fix it" sections above
|
||||
missing_titles_without_how_to_fix_its = [ s for s in missing_titles if not HOW_TO_FIX_IT_REGEX.match(s)]
|
||||
if missing_titles_without_how_to_fix_its:
|
||||
# when using the progressive education format, we need to have all its mandatory titles
|
||||
raise RuleValidationError(f'Rule {rule_language.id} is missing the "{missing_titles_without_how_to_fix_its[0]}" section')
|
||||
if missing_titles:
|
||||
# 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
|
||||
for title in h2_titles:
|
||||
@ -71,10 +68,12 @@ def validate_section_names(rule_language: LanguageSpecificRule):
|
||||
|
||||
def validate_how_to_fix_it_sections_names(rule_language: LanguageSpecificRule, h2_titles: list[str]):
|
||||
how_to_fix_it_sections = [ s for s in h2_titles if HOW_TO_FIX_IT_REGEX.match(s) ]
|
||||
if not how_to_fix_it_sections:
|
||||
# No 'How to fix it' section for the rule.
|
||||
return
|
||||
if len(how_to_fix_it_sections) > 6:
|
||||
raise RuleValidationError(f'Rule {rule_language.id} has more than 6 "{HOW_TO_FIX_IT}" sections. Please ensure this limit can be increased with PM/UX teams')
|
||||
if not how_to_fix_it_sections:
|
||||
raise RuleValidationError(f'Rule {rule_language.id} is missing a "{HOW_TO_FIX_IT}" section')
|
||||
|
||||
if HOW_TO_FIX_IT in how_to_fix_it_sections and len(how_to_fix_it_sections) > 1:
|
||||
raise RuleValidationError(f'Rule {rule_language.id} is mixing "{HOW_TO_FIX_IT}" with "How to fix it in FRAMEWORK NAME" sections. Either use a single "{HOW_TO_FIX_IT}" or one or more "How to fix it in FRAMEWORK"')
|
||||
duplicate_names = [x for x in how_to_fix_it_sections if how_to_fix_it_sections.count(x) > 1] # O(n*n) is fine, given n <= 6
|
||||
@ -165,12 +164,12 @@ Are you looking for "{highlight_name(rule_language)}"?''')
|
||||
|
||||
def validate_subsections(rule_language: LanguageSpecificRule):
|
||||
for optional_section in list(OPTIONAL_SECTIONS.keys()):
|
||||
validate_subsections_for_section(rule_language, optional_section, OPTIONAL_SECTIONS[optional_section])
|
||||
for mandatory_section in list(SECTIONS.keys()):
|
||||
if mandatory_section == HOW_TO_FIX_IT:
|
||||
validate_subsections_for_section(rule_language, mandatory_section, SECTIONS[mandatory_section], section_regex=HOW_TO_FIX_IT_REGEX)
|
||||
if optional_section == HOW_TO_FIX_IT:
|
||||
validate_subsections_for_section(rule_language, optional_section, OPTIONAL_SECTIONS[optional_section], section_regex=HOW_TO_FIX_IT_REGEX)
|
||||
else:
|
||||
validate_subsections_for_section(rule_language, mandatory_section, SECTIONS[mandatory_section])
|
||||
validate_subsections_for_section(rule_language, optional_section, OPTIONAL_SECTIONS[optional_section])
|
||||
for mandatory_section in list(SECTIONS.keys()):
|
||||
validate_subsections_for_section(rule_language, mandatory_section, SECTIONS[mandatory_section])
|
||||
for subsection_with_sub_subsection in list(SUBSECTIONS.keys()):
|
||||
if subsection_with_sub_subsection == 'Code examples':
|
||||
validate_subsections_for_section(rule_language, subsection_with_sub_subsection, SUBSECTIONS[subsection_with_sub_subsection], level=4, is_duplicate_allowed=True)
|
||||
|
@ -147,12 +147,6 @@ def test_education_format_missing_mandatory_sections_validation(invalid_rule):
|
||||
with pytest.raises(RuleValidationError, match=f'Rule common:S200 is missing the "Why is this an issue\\?" section'):
|
||||
validate_section_names(rule)
|
||||
|
||||
def test_education_missing_how_to_fix_it_validation(invalid_rule):
|
||||
'''Check that missing the "How to fix it" in the education format breaks validation'''
|
||||
rule = invalid_rule('S200', 'php')
|
||||
with pytest.raises(RuleValidationError, match=f'Rule php:S200 is missing a "How to fix it" section'):
|
||||
validate_section_names(rule)
|
||||
|
||||
def test_code_examples_with_typo_validation(invalid_rule):
|
||||
'''Check that the "Code examples" subsection with a typo in the education format breaks validation'''
|
||||
rule = invalid_rule('S200', 'cobol')
|
||||
@ -165,6 +159,11 @@ def test_noncompliant_examples_with_typo_validation(invalid_rule):
|
||||
with pytest.raises(RuleValidationError, match=f'Rule apex:S200 has a "Code examples" subsection with an unallowed name: "Non-compliant example"'):
|
||||
validate_subsections(rule)
|
||||
|
||||
def test_valid_optional_how_to_fix_it_section_validation(rule_language):
|
||||
'''Check that missing the "How to fix it" section in the education format is considered valid'''
|
||||
rule = rule_language('S200', 'php')
|
||||
validate_section_names(rule)
|
||||
|
||||
def test_valid_how_to_fix_it_subsections_validation(rule_language):
|
||||
'''Check that expected format is considered valid'''
|
||||
rule = rule_language('S101', 'csharp')
|
||||
|
Loading…
x
Reference in New Issue
Block a user