Update README and validation to reflect new guidelines (#1951)
Co-authored-by: Elena Vilchik <elena.vilchik@sonarsource.com>
This commit is contained in:
parent
80ff27bc64
commit
fb4ba0d61d
@ -38,9 +38,8 @@ This format 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#L32-L39).
|
||||
* Why is this an issue?
|
||||
** What is the potential impact? (optional)
|
||||
** Noncompliant code example (optional)
|
||||
** Compliant solution (optional)
|
||||
** Exceptions (optional)
|
||||
** (any other titles) (optional)
|
||||
* How to fix it (optional)
|
||||
** Code examples
|
||||
*** Noncompliant code example
|
||||
@ -71,9 +70,13 @@ The sections and subsections for this format are defined as follows:
|
||||
A short introduction/summary of the topic, only a few sentences. +
|
||||
Goal: The title (or message) of a rule might not always be clear due to its shortness, and this should make it clear to our users what the issue is about. Experienced developers will immediately understand what it is about, and developers not yet familiar with the issue at hand can dig deeper into the `Why is this an issue?` section.
|
||||
+
|
||||
* *Why is this an issue?* (level 2 title) [Mandatory]
|
||||
* *Why is this an issue?* (level 2 title) *[Mandatory]*
|
||||
+
|
||||
Start at the basics and go into more depth to explain the concepts behind this type of issue. This is most likely the place where a lot of the content will be. +
|
||||
This is the place to tell the “story” of the rule, including the impact of leaving it unfixed. We should include code samples wherever needed to make it easier to understand
|
||||
what is going on. This can be in the form of noncompliant and compliant code in a single code box (noncompliant lines should always be highlighted with the corresponding comment
|
||||
“// Noncompliant” optionally followed by some explanation) if that is clearer. This first tab will be like a blog post style with a free format explaining what the rule is
|
||||
detecting and why. Feel free to use “What is the potential impact?” title if it makes sense, or any other titles you find useful. +
|
||||
Goal: Understand the concepts behind an issue.
|
||||
+
|
||||
* *What is the potential impact?* (level 3 title) [Optional]
|
||||
@ -85,7 +88,8 @@ 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) [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. +
|
||||
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.
|
||||
If the fix for the rule is trivial (quickfix is available, it is easily inferred from the title and/or message), this section should be omitted and the fix could be mentioned in the previous section. +
|
||||
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.
|
||||
+
|
||||
* *How does this work?* (level 3 title) [Optional]
|
||||
|
@ -29,9 +29,7 @@ HOTSPOT_SECTION_NAMES: Final[list[str]] = parse_names('docs/header_names/hotspot
|
||||
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#2-education-format)
|
||||
SECTIONS = {
|
||||
'Why is this an issue?': ['What is the potential impact?', 'Noncompliant code example', 'Compliant solution', 'Exceptions']
|
||||
}
|
||||
MANDATORY_SECTIONS = ['Why is this an issue?']
|
||||
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'],
|
||||
@ -52,11 +50,11 @@ def validate_section_names(rule_language: LanguageSpecificRule):
|
||||
descr = rule_language.description
|
||||
h2_titles = list(map(lambda x: x.text.strip(), descr.find_all('h2')))
|
||||
|
||||
education_titles = intersection(h2_titles, list(SECTIONS.keys()) + list(OPTIONAL_SECTIONS.keys()))
|
||||
education_titles = intersection(h2_titles, list(MANDATORY_SECTIONS) + list(OPTIONAL_SECTIONS.keys()))
|
||||
if education_titles:
|
||||
# Using the education format.
|
||||
validate_how_to_fix_it_sections_names(rule_language, h2_titles)
|
||||
missing_titles = difference(list(SECTIONS.keys()), education_titles)
|
||||
missing_titles = difference(list(MANDATORY_SECTIONS), education_titles)
|
||||
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')
|
||||
@ -168,8 +166,6 @@ def validate_subsections(rule_language: LanguageSpecificRule):
|
||||
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, 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)
|
||||
|
3
rspec-tools/tests/resources/rules/S200/java/rule.adoc
Normal file
3
rspec-tools/tests/resources/rules/S200/java/rule.adoc
Normal file
@ -0,0 +1,3 @@
|
||||
== Why is this an issue?
|
||||
=== What is the potential impact?
|
||||
=== Any other title
|
13
rspec-tools/tests/resources/rules/S200/java/rule.html
Normal file
13
rspec-tools/tests/resources/rules/S200/java/rule.html
Normal file
@ -0,0 +1,13 @@
|
||||
<div class="sect1">
|
||||
<h2 id="_why_is_this_an_issue">Why is this an issue?</h2>
|
||||
<div class="sectionbody">
|
||||
<div class="sect2">
|
||||
<h3 id="_what_is_the_potential_impact">What is the potential impact?</h3>
|
||||
|
||||
</div>
|
||||
<div class="sect2">
|
||||
<h3 id="_any_other_title">Any other title</h3>
|
||||
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
@ -181,3 +181,8 @@ def test_subsections_without_a_framework_in_how_to_fix_it_validation(rule_langua
|
||||
'''Check that having subsections without a framework in "How to fix it" is considered valid'''
|
||||
rule = rule_language('S200', 'cobol')
|
||||
validate_subsections(rule)
|
||||
|
||||
def test_valid_why_is_this_an_issue_subsections_validation(rule_language):
|
||||
'''Check that any substitle is considered valid in the "why is this an issue?" section'''
|
||||
rule = rule_language('S200', 'java')
|
||||
validate_subsections(rule)
|
||||
|
Loading…
x
Reference in New Issue
Block a user