RULEAPI-748: Add new workflow to update quickfix status

.github/workflows/update_quickfix_status.yml

@@ -0,0 +1,52 @@
+name: Update quick fix status
+
+on: 
+  workflow_dispatch:
+    inputs:
+      rule:
+        description: 'ID of an existing rule (e.g., S1234).'
+        required: true
+        type: string
+      language:
+        description: 'Language to be updated for the given rule, (e.g., cfamily)'
+        required: true
+        type: string
+      status:
+        description: 'The new status for quick fix (e.g., covered)'
+        required: true
+        type: choice
+        options:
+          - covered
+          - partial
+          - targeted
+          - infeasible
+          - unknown
+
+jobs:
+  update_quickfix_status:
+    name: Update quick fix status
+    runs-on: ubuntu-20.04
+    env:
+      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        persist-credentials: true
+        ref: master
+        path: 'rspec'
+
+    - uses: actions/setup-python@v2
+      with:
+        python-version: '3.9'
+
+    - name: 'Install Pipenv'
+      run: |
+        pip install pipenv
+
+    - name: 'Install rspec-tools'
+      working-directory: 'rspec/rspec-tools'
+      run: pipenv install
+
+    - name: 'Update quickfix status'
+      working-directory: 'rspec/rspec-tools'
+      run: pipenv run rspec-tools update-quickfix-status --user ${{ github.actor }} --rule "${{ github.event.inputs.rule }}" --language "${{ github.event.inputs.language }}" --status "${{ github.event.inputs.status }}"

docs/metadata.adoc

@@ -1,3 +1,7 @@
+ifdef::env-github[]
+:tip-caption: :bulb:
+:note-caption: :information_source:
+endif::[]
 = Rule Metadata
 
 This document describes how `+metadata.json+` should be structured.
@@ -21,3 +25,8 @@ For instance, on one hand, if a rule detects two functions that are dangerous to
 
 On the other hand, if a quick fix could be easily proposed for both `A` and `B`, but the fix location might be inside a macro expansion, or in a different file from the issue location (and hence not feasible given the current SonarLint capabilities), this should not prevent the rule from being tagged as `covered`.
 ====
+
+[TIP]
+====
+You can update the quickfix field using this GitHub Workflow: https://github.com/SonarSource/rspec/actions/workflows/update_quickfix_status.yml
+====

rspec-tools/rspec_tools/cli.py

@@ -5,11 +5,11 @@ from pathlib import Path
 import click
 from rspec_tools.checklinks import check_html_links
 from rspec_tools.errors import RuleNotFoundError, RuleValidationError
-from rspec_tools.create_rule import create_new_rule, add_language_to_rule
 from rspec_tools.rules import RulesRepository, LanguageSpecificRule
 from rspec_tools.validation.metadata import validate_rule_metadata
 from rspec_tools.validation.description import validate_section_names, validate_section_levels, validate_parameters, validate_source_language
 from rspec_tools.coverage import update_coverage_for_all_repos, update_coverage_for_repo, update_coverage_for_repo_version
+import rspec_tools.create_rule
 
 from rspec_tools.notify_failure_on_slack import notify_slack
 
@@ -38,7 +38,7 @@ def check_links(d):
 def create_rule(languages: str, user: Optional[str]):
   '''Create a new rule.'''
   token = os.environ.get('GITHUB_TOKEN')
-  create_new_rule(languages, token, user)
+  rspec_tools.create_rule.create_new_rule(languages, token, user)
 
 @cli.command()
 @click.option('--language', required=True)
@@ -47,7 +47,17 @@ def create_rule(languages: str, user: Optional[str]):
 def add_lang_to_rule(language: str, rule: str, user: Optional[str]):
   '''Add a new language to rule.'''
   token = os.environ.get('GITHUB_TOKEN')
-  add_language_to_rule(language, rule, token, user)
+  rspec_tools.create_rule.add_language_to_rule(language, rule, token, user)
+
+@cli.command()
+@click.option('--language', required=True)
+@click.option('--rule', required=True)
+@click.option('--status', required=True)
+@click.option('--user', required=False)
+def update_quickfix_status(language: str, rule: str, status: str, user: Optional[str]):
+  '''Update the status of quick fix for the given rule/language'''
+  token = os.environ.get('GITHUB_TOKEN')
+  rspec_tools.create_rule.update_rule_quickfix_status(language, rule, status, token, user)
 
 @cli.command()
 @click.argument('rules', nargs=-1, required=True)

rspec-tools/rspec_tools/create_rule.py

@@ -1,24 +1,30 @@
-from rspec_tools.errors import InvalidArgumentError
-import click
+import json
+import os
 import tempfile
-import fs
+from contextlib import contextmanager
+from pathlib import Path
+from typing import Callable, Final, Iterable, Optional
+
+import click
 from git import Repo
-from git.remote import PushInfo
 from github import Github
 from github.PullRequest import PullRequest
-from pathlib import Path
-from typing import Final, Iterable, Optional, Callable
-from contextlib import contextmanager
-from rspec_tools.utils import parse_and_validate_language_list, get_labels_for_languages, validate_language, get_label_for_language, resolve_rule, swap_metadata_files, is_empty_metadata
 
-from rspec_tools.utils import copy_directory_content, LANG_TO_SOURCE
+from rspec_tools.errors import InvalidArgumentError
+from rspec_tools.utils import (LANG_TO_SOURCE, copy_directory_content,
+                               get_label_for_language,
+                               get_labels_for_languages, is_empty_metadata,
+                               parse_and_validate_language_list, resolve_rule,
+                               swap_metadata_files, validate_language)
+
 
 def build_github_repository_url(token: str, user: Optional[str]):
-  'Builds the rspec repository url'
+  '''Builds the rspec repository url'''
+  repo = os.environ.get('GITHUB_REPOSITORY', 'SonarSource/rspec')
   if user:
-    return f'https://{user}:{token}@github.com/SonarSource/rspec.git'
+    return f'https://{user}:{token}@github.com/{repo}.git'
   else:
-    return f'https://{token}@github.com/SonarSource/rspec.git'
+    return f'https://{token}@github.com/{repo}.git'
 
 def extract_repository_name(url):
   url_end = url.split('/')[-2:]
@@ -32,33 +38,45 @@ def auto_github(token: str) -> Callable[[Optional[str]], Github]:
       return Github(token)
   return ret
 
-def create_new_rule(languages: str, token: str, user: Optional[str]):
+def _get_url_and_config(token: str, user: Optional[str]):
   url = build_github_repository_url(token, user)
   config = {}
   if user:
     config['user.name'] = user
     config['user.email'] = f'{user}@users.noreply.github.com'
-  lang_list = parse_and_validate_language_list(languages)
-  label_list = get_labels_for_languages(lang_list)
 
+  return url, config
+
+def _get_valid_label_for_language(language: str):
+  validate_language(language)
+  return get_label_for_language(language)
+
+@contextmanager
+def _rule_creator(token: str, user: Optional[str]):
+  url, config = _get_url_and_config(token, user)
   with tempfile.TemporaryDirectory() as tmpdirname:
     rule_creator = RuleCreator(url, tmpdirname, config)
+    yield rule_creator
+
+def create_new_rule(languages: str, token: str, user: Optional[str]):
+  lang_list = parse_and_validate_language_list(languages)
+  label_list = get_labels_for_languages(lang_list)
+  with _rule_creator(token, user) as rule_creator:
     rule_number = rule_creator.reserve_rule_number()
     rule_creator.create_new_rule_pull_request(auto_github(token), rule_number, lang_list, label_list, user=user)
 
 def add_language_to_rule(language: str, rule: str, token: str, user: Optional[str]):
-  url = build_github_repository_url(token, user)
-  config = {}
-  if user:
-    config['user.name'] = user
-    config['user.email'] = f'{user}@users.noreply.github.com'
-  validate_language(language)
-  label = get_label_for_language(language)
+  label = _get_valid_label_for_language(language)
   rule_number = resolve_rule(rule)
-  with tempfile.TemporaryDirectory() as tmpdirname:
-    rule_creator = RuleCreator(url, tmpdirname, config)
+  with _rule_creator(token, user) as rule_creator:
     rule_creator.add_language_pull_request(auto_github(token), rule_number, language, label, user=user)
 
+def update_rule_quickfix_status(language: str, rule: str, status: str, token: str, user: Optional[str]):
+  label = _get_valid_label_for_language(language)
+  rule_number = resolve_rule(rule)
+  with _rule_creator(token, user) as rule_creator:
+    rule_creator.update_quickfix_status_pull_request(auto_github(token), rule_number, language, status, label, user)
+
 class RuleCreator:
   ''' Create a new Rule in a repository following the official Github 'rspec' repository structure.'''
   MASTER_BRANCH: Final[str] = 'master'
@@ -144,6 +162,39 @@ class RuleCreator:
     self.repository.git.push('origin', branch_name)
     return branch_name
 
+  def update_quickfix_status_branch(self, title: str, rule_number: int, language: str, status: str) -> str:
+    '''Update the given rule/language quick fix metadata field.'''
+    branch_name = f'rule/S{rule_number}-{language}-quickfix'
+    with self._current_git_branch(self.MASTER_BRANCH, branch_name):
+      self._update_quickfix_status(rule_number, language, status)
+      self.repository.git.add('--all')
+      self.repository.index.commit(title)
+    self.repository.git.push('origin', branch_name)
+    return branch_name
+
+  def _get_generic_quickfix_status(self, rule_number: int):
+    DEFAULT = 'unknown'
+    generic_metadata_path = Path(self.repository.working_dir, 'rules', f'S{rule_number}', 'metadata.json')
+    if not generic_metadata_path.is_file():
+      return DEFAULT
+    generic_metadata = json.loads(generic_metadata_path.read_text())
+    return generic_metadata.get('quickfix', DEFAULT)
+
+  def _update_quickfix_status(self, rule_number: int, language: str, status: str):
+    metadata_path = Path(self.repository.working_dir, 'rules', f'S{rule_number}', language, 'metadata.json')
+    if not metadata_path.is_file():
+      raise InvalidArgumentError(f'{metadata_path} does not exist or is not a file')
+
+    metadata = json.loads(metadata_path.read_text())
+    generic_status = self._get_generic_quickfix_status(rule_number)
+    if status == metadata.get('quickfix', generic_status):
+      raise InvalidArgumentError(f'{metadata_path} has already the same status {status}')
+
+    metadata['quickfix'] = status
+    # When generating the JSON, ensure forward slashes are escaped. See RULEAPI-750.
+    json_string = json.dumps(metadata, indent=2).replace("/", "\\/")
+    metadata_path.write_text(json_string)
+
   def _fill_in_the_blanks_in_the_template(self, rule_dir: Path, rule_number: int):
     for rule_item in rule_dir.glob('**/*'):
       if rule_item.is_file():
@@ -228,6 +279,21 @@ class RuleCreator:
       user
     )
 
+  def update_quickfix_status_pull_request(self, github_api: Callable[[Optional[str]], Github], rule_number: int, language: str, status: str, label: str, user: Optional[str]):
+    title = f'Modify rule S{rule_number}: mark quick fix as "{status}"'
+    branch_name = self.update_quickfix_status_branch(title, rule_number, language, status)
+    click.echo(f'Created rule branch {branch_name}')
+    return self._create_pull_request(
+      github_api,
+      branch_name,
+      title,
+      f'''See the original rule [here](https://sonarsource.github.io/rspec/#/rspec/S{rule_number}/{language}).
+
+The rule won't be updated until this PR is merged, see [RULEAPI-655](https://jira.sonarsource.com/browse/RULEAPI-655)''',
+      [label],
+      user
+    )
+
   @contextmanager
   def _current_git_branch(self, base_branch: str, new_branch: Optional[str] = None):
     '''Checkout a given branch before yielding, then revert to the previous branch.'''

rspec-tools/rspec_tools/utils.py

@@ -1,5 +1,6 @@
 from rspec_tools.errors import InvalidArgumentError
 from pathlib import Path
+from typing import List
 import shutil
 import re
 import tempfile
@@ -112,20 +113,21 @@ def get_mapped_languages():
   Necessary to make sure all valid languages are mapped (see test_utils.py).'''
   return LANG_TO_LABEL.keys();
 
+def _validate_languages(languages: List[str]):
+  valid_langs = load_valid_languages()
+  for lang in languages:
+    if lang not in valid_langs:
+      raise InvalidArgumentError(f"Unsupported language: \"{lang}\". See {SUPPORTED_LANGUAGES_FILENAME} for the list of supported languages.")
+
 def parse_and_validate_language_list(languages):
   lang_list = [lang.strip() for lang in languages.split(',')]
   if len(languages.strip()) == 0 or len(lang_list) == 0:
     raise InvalidArgumentError('Invalid argument for "languages". At least one language should be provided.')
-  valid_langs = load_valid_languages()
-  for lang in lang_list:
-    if lang not in valid_langs:
-      raise InvalidArgumentError(f"Unsupported language: \"{lang}\". See {SUPPORTED_LANGUAGES_FILENAME} for the list of supported languages.")
+  _validate_languages(lang_list)
   return lang_list
 
 def validate_language(language):
-  valid_langs = load_valid_languages()
-  if language not in valid_langs:
-    raise InvalidArgumentError(f"Unsupported language: \"{language}\". See {SUPPORTED_LANGUAGES_FILENAME} for the list of supported languages.")
+  _validate_languages([language])
 
 def get_labels_for_languages(lang_list):
   labels = [LANG_TO_LABEL[lang] for lang in lang_list]

rspec-tools/tests/resources/rules/S100/java/metadata.json

@@ -1,3 +1,3 @@
 {
-  
+  "quickfix": "targeted"
 }

rspec-tools/tests/test_cli.py

@@ -1,13 +1,31 @@
+import os
+import re
 from pathlib import Path
 from typing import List
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 
 from click.testing import CliRunner
 from rspec_tools.cli import cli
 from rspec_tools.rules import RulesRepository
+import rspec_tools
+
+
+class TestCLIUpdateQuickfixStatus:
+  '''Unit test for quickfix status update through Command Line Interface.'''
+
+  @patch.dict(os.environ, {'GITHUB_TOKEN': 'TOKEN'})
+  @patch('rspec_tools.create_rule.update_rule_quickfix_status')
+  def test_basic_cli_usage(self, mock):
+    arguments = [
+      'update-quickfix-status',
+      '--language', 'langA',
+      '--rule', 'ruleX',
+      '--status', 'myStatus',
+      '--user', 'bob',
+    ]
+    CliRunner().invoke(cli, arguments)
+    mock.assert_called_once_with('langA', 'ruleX', 'myStatus', 'TOKEN', 'bob')
 
-import os
-import re
 
 class TestCLIValidateRulesMetadata:
   '''Unit tests for metadata validation through Command Line Interface.'''

rspec-tools/tests/test_create_rule.py

@@ -1,14 +1,18 @@
-from git import Repo, Head
-from rspec_tools.errors import InvalidArgumentError
+import json
+import os
+import shutil
 from pathlib import Path
 from typing import Optional
 from unittest.mock import Mock, patch
-import pytest
-import shutil
-import os
 
-from rspec_tools.create_rule import RuleCreator, create_new_rule, add_language_to_rule
-from rspec_tools.utils import is_empty_metadata, LANG_TO_SOURCE
+import pytest
+from git import Head, Repo
+from rspec_tools.create_rule import (RuleCreator, add_language_to_rule,
+                                     create_new_rule,
+                                     update_rule_quickfix_status)
+from rspec_tools.errors import InvalidArgumentError
+from rspec_tools.utils import LANG_TO_SOURCE, is_empty_metadata
+
 
 @pytest.fixture
 def git_config():
@@ -81,6 +85,121 @@ def read_counter_file(repo):
   return counter_path.read_text()
 
 
+def test_update_quickfix_status_branch1(rule_creator: RuleCreator, mock_rspec_repo: Repo):
+  '''Test update_quickfix_status_branch when quickfix field is not present in language-specific metadata'''
+  rule_number = 100
+  language = 'cfamily'
+
+  sub_path = Path('rules', f'S{rule_number}', language, 'metadata.json')
+  metadata_path = Path(mock_rspec_repo.working_dir) / sub_path
+  mock_rspec_repo.git.checkout('master')
+  initial_metadata = json.loads(metadata_path.read_text())
+  assert 'quickfix' not in initial_metadata # It is in the parent metadata.json file.
+
+  branch = rule_creator.update_quickfix_status_branch('Some title', rule_number, language, 'covered')
+  mock_rspec_repo.git.checkout(branch)
+  new_metadata = json.loads(metadata_path.read_text())
+
+  # Verify that only the quickfix status is introduced.
+  assert len(initial_metadata.keys()) + 1 == len(new_metadata.keys())
+  assert 'quickfix' in new_metadata
+  assert 'covered' == new_metadata['quickfix']
+  for key in initial_metadata:
+    assert initial_metadata[key] == new_metadata[key]
+
+  # Ensure only one file is modified.
+  modified_files = mock_rspec_repo.git.diff('master', '--name-only').strip()
+  assert modified_files == str(sub_path)
+
+
+def test_update_quickfix_status_branch2(rule_creator: RuleCreator, mock_rspec_repo: Repo):
+  '''Test update_quickfix_status_branch when quickfix field is already present in language-specific metadata'''
+  rule_number = 100
+  language = 'java'
+
+  sub_path = Path('rules', f'S{rule_number}', language, 'metadata.json')
+  metadata_path = Path(mock_rspec_repo.working_dir) / sub_path
+  mock_rspec_repo.git.checkout('master')
+  initial_metadata = json.loads(metadata_path.read_text())
+  assert 'quickfix' in initial_metadata
+  assert initial_metadata['quickfix'] == 'targeted'
+
+  branch = rule_creator.update_quickfix_status_branch('Some title', rule_number, language, 'covered')
+  mock_rspec_repo.git.checkout(branch)
+  new_metadata = json.loads(metadata_path.read_text())
+
+  # Verify that only the quickfix status is updated.
+  assert len(initial_metadata.keys()) == len(new_metadata.keys())
+  assert 'quickfix' in new_metadata
+  assert 'covered' == new_metadata['quickfix']
+  for key in initial_metadata:
+    if key != 'quickfix':
+      assert initial_metadata[key] == new_metadata[key]
+
+  # Ensure only one file is modified.
+  modified_files = mock_rspec_repo.git.diff('master', '--name-only').strip()
+  assert modified_files == str(sub_path)
+
+
+def test_update_quickfix_status_branch3(rule_creator: RuleCreator, mock_rspec_repo: Repo):
+  '''Test update_quickfix_status_branch when new and old quickfix status are the same'''
+  rule_number = 100
+  language = 'java'
+
+  metadata_path = Path(mock_rspec_repo.working_dir, 'rules', f'S{rule_number}', language, 'metadata.json')
+  mock_rspec_repo.git.checkout('master')
+  initial_metadata = json.loads(metadata_path.read_text())
+  assert 'quickfix' in initial_metadata
+  assert initial_metadata['quickfix'] == 'targeted'
+
+  with pytest.raises(InvalidArgumentError):
+    rule_creator.update_quickfix_status_branch('Some title', rule_number, language, 'targeted')
+
+
+def test_update_quickfix_status_branch4(rule_creator: RuleCreator, mock_rspec_repo: Repo):
+  '''Test update_quickfix_status_branch when the rule does not exist'''
+  rule_number = 404
+  language = 'java'
+
+  metadata_path = Path(mock_rspec_repo.working_dir, 'rules', f'S{rule_number}', language, 'metadata.json')
+  mock_rspec_repo.git.checkout('master')
+  assert not metadata_path.exists()
+
+  with pytest.raises(InvalidArgumentError):
+    rule_creator.update_quickfix_status_branch('Some title', rule_number, language, 'targeted')
+
+
+def test_update_quickfix_status_pull_request(rule_creator: RuleCreator):
+  '''Test update_quickfix_status_pull_request adds the right user and label.'''
+  ghMock = Mock()
+  ghRepoMock = Mock()
+  pullMock = Mock()
+  ghRepoMock.create_pull = Mock(return_value=pullMock)
+  ghMock.get_repo = Mock(return_value=ghRepoMock)
+  def mockGithub(user: Optional[str]):
+    return ghMock
+
+  rule_creator.update_quickfix_status_pull_request(mockGithub, 100, 'cfamily', 'covered', 'label-fraicheur', 'testuser')
+  ghRepoMock.create_pull.assert_called_once();
+  assert ghRepoMock.create_pull.call_args.kwargs['title'].startswith('Modify rule S100')
+  pullMock.add_to_assignees.assert_called_with('testuser')
+  pullMock.add_to_labels.assert_called_with('label-fraicheur')
+
+
+@patch('rspec_tools.create_rule.RuleCreator')
+def test_update_rule_quickfix_status(mockRuleCreator):
+  '''Test update_rule_quickfix_status uses the expected implementation.'''
+  mockRuleCreator.return_value = Mock()
+  mockRuleCreator.return_value.update_quickfix_status_pull_request = Mock()
+  prMock = mockRuleCreator.return_value.update_quickfix_status_pull_request
+  update_rule_quickfix_status('cfamily', 'S100', 'covered', 'my token', 'testuser')
+  prMock.assert_called_once()
+  assert prMock.call_args.args[1] == 100
+  assert prMock.call_args.args[2] == 'cfamily'
+  assert prMock.call_args.args[3] == 'covered'
+  assert prMock.call_args.args[4] == 'cfamily'
+
+
 def test_create_new_multi_lang_rule_branch(rule_creator: RuleCreator, mock_rspec_repo: Repo):
   '''Test create_new_rule_branch for a multi-language rule.'''
   rule_number = rule_creator.reserve_rule_number()