Add check that each section is used only once

There was already a check for section duplication, but only in "How to
fix it". This changes the test to cover all sections.
And fixing the rules that this new validation fails on.

Also making test_modify_rule.py run on Windows.

---------

Co-authored-by: Christophe Zürn <36889251+christophe-zurn-sonarsource@users.noreply.github.com>

rspec-tools/rspec_tools/validation/description.py

@@ -1,3 +1,4 @@
+import collections
 from bs4 import BeautifulSoup
 from pathlib import Path
 from typing import Final
@@ -39,10 +40,16 @@ SUBSECTIONS = {
   'Code examples': ['Noncompliant code example', 'Compliant solution']
 }
 
-def intersection(lst1, lst2):
-  return list(set(lst1).intersection(lst2))
-def difference(lst1, lst2):
-  return list(set(lst1) - set(lst2))
+def validate_duplications(h2_titles, rule_language):
+  as_set = set(h2_titles)
+  if len(as_set) != len(h2_titles):
+    duplicates = [x for x in h2_titles if h2_titles.count(x) > 1]
+    raise RuleValidationError(f'Rule {rule_language.id} has duplicated {set(duplicates)} sections')
+
+def intersection(list1, list2):
+  return list(set(list1).intersection(list2))
+def difference(list1, list2):
+  return list(set(list1) - set(list2))
 
 def validate_section_names(rule_language: LanguageSpecificRule):
   """Validates all h2-level section names"""
@@ -50,6 +57,8 @@ def validate_section_names(rule_language: LanguageSpecificRule):
   descr = rule_language.description
   h2_titles = list(map(lambda x: x.text.strip(), descr.find_all('h2')))
 
+  validate_duplications(h2_titles, rule_language)
+
   education_titles = intersection(h2_titles, list(MANDATORY_SECTIONS) + list(OPTIONAL_SECTIONS.keys()))
   if education_titles:
     # Using the education format.
@@ -74,9 +83,6 @@ def validate_how_to_fix_it_sections_names(rule_language: LanguageSpecificRule, h
 
   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
-  if 0 < len(duplicate_names):
-    raise RuleValidationError(f'Rule {rule_language.id} has duplicate "{HOW_TO_FIX_IT}" sections {set(duplicate_names)}')
   for section_name in how_to_fix_it_sections:
     validate_how_to_fix_it_framework(section_name, rule_language)
 

rspec-tools/tests/resources/invalid-rules/S101/javascript/rule.html

@@ -11,37 +11,37 @@
 </div>
 </div>
 <div class="sect1">
-<h2 id="_how_to_fix_it_in_express_js_2">How to fix it in Express.js</h2>
+<h2 id="_how_to_fix_it_in_node_js">How to fix it in Node.js</h2>
 <div class="sectionbody">
 
 </div>
 </div>
 <div class="sect1">
-<h2 id="_how_to_fix_it_in_express_js_3">How to fix it in Express.js</h2>
+<h2 id="_how_to_fix_it_in_ssh2">How to fix it in SSH2</h2>
 <div class="sectionbody">
 
 </div>
 </div>
 <div class="sect1">
-<h2 id="_how_to_fix_it_in_express_js_4">How to fix it in Express.js</h2>
+<h2 id="_how_to_fix_it_in_mongo_db">How to fix it in MongoDB</h2>
 <div class="sectionbody">
 
 </div>
 </div>
 <div class="sect1">
-<h2 id="_how_to_fix_it_in_express_js_5">How to fix it in Express.js</h2>
+<h2 id="_how_to_fix_it_in_mongoose">How to fix it in Mongoose</h2>
 <div class="sectionbody">
 
 </div>
 </div>
 <div class="sect1">
-<h2 id="_how_to_fix_it_in_express_js_6">How to fix it in Express.js</h2>
+<h2 id="_how_to_fix_it_in_sequelize">How to fix it in Sequelize</h2>
 <div class="sectionbody">
 
 </div>
 </div>
 <div class="sect1">
-<h2 id="_how_to_fix_it_in_express_js_7">How to fix it in Express.js</h2>
+<h2 id="_how_to_fix_it_in_knex">How to fix it in Knex</h2>
 <div class="sectionbody">
 
 </div>

rspec-tools/tests/test_modify_rule.py

@@ -40,7 +40,7 @@ def test_update_quickfix_status_branch1(rule_editor: RuleEditor, mock_git_rspec_
 
   # Ensure only one file is modified.
   modified_files = mock_git_rspec_repo.git.diff('master', '--name-only').strip()
-  assert modified_files == str(sub_path)
+  assert Path(modified_files) == sub_path
 
 
 def test_update_quickfix_status_branch2(rule_editor: RuleEditor, mock_git_rspec_repo: Repo):
@@ -69,7 +69,7 @@ def test_update_quickfix_status_branch2(rule_editor: RuleEditor, mock_git_rspec_
 
   # Ensure only one file is modified.
   modified_files = mock_git_rspec_repo.git.diff('master', '--name-only').strip()
-  assert modified_files == str(sub_path)
+  assert Path(modified_files) == sub_path
 
 
 def test_update_quickfix_status_branch3(rule_editor: RuleEditor, mock_git_rspec_repo: Repo):

rspec-tools/tests/validation/test_description_validation.py

@@ -107,10 +107,10 @@ def test_single_how_to_fix_it_allowed_validation(invalid_rule):
   with pytest.raises(RuleValidationError, match=f'Rule abap:S200 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"'):
     validate_section_names(rule)
 
-def test_duplicate_frameworks_in_how_to_fix_it_validation(invalid_rule):
-  '''Check that duplicate "How to fix it" subsections for the same framework breaks validation'''
+def test_duplicate_h2_sections_validation(invalid_rule):
+  '''Check that duplicate H2 sections breaks validation'''
   rule = invalid_rule('S200', 'javascript')
-  with pytest.raises(RuleValidationError, match='Rule javascript:S200 has duplicate "How to fix it" sections {\'How to fix it in Razor\'}'):
+  with pytest.raises(RuleValidationError, match='Rule javascript:S200 has duplicated {\'How to fix it in Razor\'} sections'):
     validate_section_names(rule)
 
 def test_wrong_format_how_to_fix_it_section_validation(invalid_rule):

rules/S2615/rule.adoc

@@ -12,11 +12,3 @@ public void formattedLog(String format, String message) {
   LOGGER.info(logLine);
 }
 ----
-
-
-== Resources
-
-* https://owasp.org/www-project-top-ten/2017/A1_2017-Injection[OWASP Top 10 2017 Category A1] - Injection
-* https://cwe.mitre.org/data/definitions/134[MITRE, CWE-134] - Use of Externally-Controlled Format String
-* https://www.sans.org/top25-software-errors/#cat2[SANS Top 25] - Risky Resource Management
-

rules/S2615/swift/rule.adoc

@@ -1,5 +1,11 @@
 include::../rule.adoc[]
 
+== Resources
+
+* https://owasp.org/www-project-top-ten/2017/A1_2017-Injection[OWASP Top 10 2017 Category A1] - Injection
+* https://cwe.mitre.org/data/definitions/134[MITRE, CWE-134] - Use of Externally-Controlled Format String
+* https://www.sans.org/top25-software-errors/#cat2[SANS Top 25] - Risky Resource Management
+
 ifdef::env-github,rspecator-view[]
 
 '''

rules/S4663/description.adoc

@@ -1,5 +1,3 @@
-== Why is this an issue?
-
 Empty comments like the following don't improve readability and might indicate an oversight.
 
 include::{example}[]

rules/S4663/ruby/rule.adoc

@@ -1,3 +1,5 @@
+== Why is this an issue?
+
 :example: ruby/code-example.adoc
 
 include::../description.adoc[]

rules/S4663/rule.adoc

@@ -1,3 +1,5 @@
+== Why is this an issue?
+
 :example: common/code-example.adoc
 
 include::description.adoc[]

rules/S4663/vbnet/rule.adoc

@@ -1,3 +1,5 @@
+== Why is this an issue?
+
 :example: vbnet/code-example.adoc
 
 include::../description.adoc[]