mirror of
https://gcc.gnu.org/git/gcc.git
synced 2026-02-21 19:35:28 -05:00
sarif output: add descriptions to fix-it hints (§3.55.2) [PR121986]
SARIF "fix" objects SHOULD have a "description" property (§3.55.2) that describes the proposed fix, but currently GCC's SARIF output doesn't support this, and we don't capture this anywhere internally as we build fix-it hints in the compiler. Currently we can have zero or more instances of fixit_hint associated with a diagnostic, each representing an edit of a range of the source code. Ideally we would have an internal API that allowed for associating multiple fixes with a diagnostic, each with a description worded in terms of the source language (e.g. "Fix 'colour' mispelling of field 'color'"), and each consisting of multiple edited ranges. For now, this patch extends the sarif output sink so that it autogenerates descriptions of fix-it hints for simple cases of insertion, deletion, and replacement of a single range (e.g. "Replace 'colour' with 'color'"). gcc/ChangeLog: PR diagnostics/121986 * diagnostics/sarif-sink.cc: Include "intl.h". (sarif_builder::make_message_describing_fix_it_hint): New. (sarif_builder::make_fix_object): Attempt to auto-generate a description for fix-it hints. gcc/testsuite/ChangeLog: PR diagnostics/121986 * gcc.dg/sarif-output/extra-semicolon.c: New test. * gcc.dg/sarif-output/extra-semicolon.py: New test. * gcc.dg/sarif-output/missing-semicolon.py: Verify the description of the insertion fix-it hint. * libgdiagnostics.dg/test-fix-it-hint-c.py: Verify the description of the replacement fix-it hint. libcpp/ChangeLog: PR diagnostics/121986 * include/rich-location.h (fixit_hint::deletion_p): New accessor. (fixit_hint::replacement_p): New accessor. Signed-off-by: David Malcolm <dmalcolm@redhat.com>
This commit is contained in:
@@ -54,6 +54,7 @@ along with GCC; see the file COPYING3. If not see
|
||||
#include "demangle.h"
|
||||
#include "backtrace.h"
|
||||
#include "xml.h"
|
||||
#include "intl.h"
|
||||
|
||||
namespace diagnostics {
|
||||
|
||||
@@ -948,6 +949,8 @@ private:
|
||||
int start_line,
|
||||
int end_line,
|
||||
const content_renderer *r) const;
|
||||
std::unique_ptr<sarif_message>
|
||||
make_message_describing_fix_it_hint (const fixit_hint &hint) const;
|
||||
std::unique_ptr<sarif_fix>
|
||||
make_fix_object (const rich_location &rich_loc);
|
||||
std::unique_ptr<sarif_artifact_change>
|
||||
@@ -3747,6 +3750,65 @@ maybe_make_artifact_content_object (const char *filename,
|
||||
return artifact_content_obj;
|
||||
}
|
||||
|
||||
/* Attempt to generate a "message" object describing a fix-it hint,
|
||||
or null if there was a problem. */
|
||||
|
||||
std::unique_ptr<sarif_message>
|
||||
sarif_builder::
|
||||
make_message_describing_fix_it_hint (const fixit_hint &hint) const
|
||||
{
|
||||
pretty_printer pp;
|
||||
if (hint.insertion_p ())
|
||||
pp_printf (&pp, G_("Insert %qs"), hint.get_string ());
|
||||
else
|
||||
{
|
||||
/* Try to get prior content. */
|
||||
expanded_location start = expand_location (hint.get_start_loc ());
|
||||
expanded_location next_loc = expand_location (hint.get_next_loc ());
|
||||
if (start.file != next_loc.file)
|
||||
return nullptr;
|
||||
if (start.line != next_loc.line)
|
||||
return nullptr;
|
||||
if (start.column == 0)
|
||||
return nullptr;
|
||||
if (next_loc.column == 0)
|
||||
return nullptr;
|
||||
|
||||
const int start_offset = start.column - 1;
|
||||
const int next_offset = next_loc.column - 1;
|
||||
if (next_offset <= start_offset)
|
||||
return nullptr;
|
||||
|
||||
size_t victim_len = next_offset - start_offset;
|
||||
|
||||
char_span existing_line = get_context ()
|
||||
.get_file_cache ()
|
||||
.get_source_line (start.file, start.line);
|
||||
if (!existing_line)
|
||||
return nullptr;
|
||||
|
||||
label_text existing_text
|
||||
= label_text::take (existing_line.subspan (start_offset,
|
||||
victim_len).xstrdup ());
|
||||
|
||||
if (hint.deletion_p ())
|
||||
{
|
||||
// Removal
|
||||
pp_printf (&pp, G_("Delete %qs"),
|
||||
existing_text.get ());
|
||||
}
|
||||
else
|
||||
{
|
||||
// Replacement
|
||||
gcc_assert (hint.replacement_p ());
|
||||
pp_printf (&pp, G_("Replace %qs with %qs"),
|
||||
existing_text.get (),
|
||||
hint.get_string ());
|
||||
}
|
||||
}
|
||||
return make_message_object (pp_formatted_text (&pp));
|
||||
}
|
||||
|
||||
/* Make a "fix" object (SARIF v2.1.0 section 3.55) for RICHLOC. */
|
||||
|
||||
std::unique_ptr<sarif_fix>
|
||||
@@ -3762,6 +3824,18 @@ sarif_builder::make_fix_object (const rich_location &richloc)
|
||||
fix_obj->set<json::array> ("artifactChanges",
|
||||
std::move (artifact_change_arr));
|
||||
|
||||
// 3.55.2 "description" property
|
||||
/* Attempt to generate a description. We can only do this
|
||||
if there was a single hint. */
|
||||
if (richloc.get_num_fixit_hints () == 1)
|
||||
{
|
||||
const fixit_hint *hint = richloc.get_fixit_hint (0);
|
||||
gcc_assert (hint);
|
||||
if (auto desc_msg = make_message_describing_fix_it_hint (*hint))
|
||||
fix_obj->set<sarif_message> ("description",
|
||||
std::move (desc_msg));
|
||||
}
|
||||
|
||||
return fix_obj;
|
||||
}
|
||||
|
||||
|
||||
16
gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.c
Normal file
16
gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.c
Normal file
@@ -0,0 +1,16 @@
|
||||
/* { dg-do compile } */
|
||||
/* { dg-options "-fdiagnostics-format=sarif-file -Wpedantic" } */
|
||||
|
||||
/* Verify SARIF output for a deletion fix-it hint. */
|
||||
|
||||
struct foo
|
||||
{
|
||||
int color;;
|
||||
};
|
||||
|
||||
/* Verify that some JSON was written to a file with the expected name:
|
||||
{ dg-final { verify-sarif-file } } */
|
||||
|
||||
/* Use a Python script to verify various properties about the generated
|
||||
.sarif file:
|
||||
{ dg-final { run-sarif-pytest extra-semicolon.c "extra-semicolon.py" } } */
|
||||
37
gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.py
Normal file
37
gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.py
Normal file
@@ -0,0 +1,37 @@
|
||||
from sarif import *
|
||||
|
||||
import pytest
|
||||
|
||||
@pytest.fixture(scope='function', autouse=True)
|
||||
def sarif():
|
||||
return sarif_from_env()
|
||||
|
||||
def test_deletion_fixit(sarif):
|
||||
runs = sarif['runs']
|
||||
run = runs[0]
|
||||
results = run['results']
|
||||
|
||||
# We expect a single error with a secondary location and a fix-it hint.
|
||||
#
|
||||
# The textual form of the diagnostic would look like this:
|
||||
# . PATH/extra-semicolon.c:8:13: warning: extra semicolon in struct or union specified [-Wpedantic]
|
||||
# . 8 | int color;;
|
||||
# . | ^
|
||||
# . | -
|
||||
assert len(results) == 1
|
||||
|
||||
result = results[0]
|
||||
assert result['level'] == 'warning'
|
||||
assert result['message']['text'] == "extra semicolon in struct or union specified"
|
||||
|
||||
# We expect one fix-it hint representing a deletion of ';'
|
||||
assert len(result['fixes']) == 1
|
||||
fix = result['fixes'][0]
|
||||
assert fix['description']['text'] == "Delete ';'"
|
||||
assert len(fix['artifactChanges']) == 1
|
||||
change = fix['artifactChanges'][0]
|
||||
replacement = change['replacements'][0]
|
||||
assert replacement['deletedRegion']['startLine'] == 8
|
||||
assert replacement['deletedRegion']['startColumn'] == 13
|
||||
assert replacement['deletedRegion']['endColumn'] == 14
|
||||
assert replacement['insertedContent']['text'] == ''
|
||||
@@ -79,8 +79,10 @@ def test_location_relationships(sarif):
|
||||
|
||||
# We expect one fix-it hint representing an insertion of ';'
|
||||
assert len(result['fixes']) == 1
|
||||
assert len(result['fixes'][0]['artifactChanges']) == 1
|
||||
change = result['fixes'][0]['artifactChanges'][0]
|
||||
fix = result['fixes'][0]
|
||||
assert fix['description']['text'] == "Insert ';'"
|
||||
assert len(fix['artifactChanges']) == 1
|
||||
change = fix['artifactChanges'][0]
|
||||
assert change['artifactLocation']['uri'].endswith('missing-semicolon.c')
|
||||
assert len(change['replacements']) == 1
|
||||
replacement = change['replacements'][0]
|
||||
|
||||
@@ -37,6 +37,7 @@ def test_sarif_output_with_fixes(sarif):
|
||||
|
||||
assert len(results[0]['fixes']) == 1
|
||||
fix = results[0]['fixes'][0]
|
||||
assert fix['description']['text'] == "Replace 'colour' with 'color'"
|
||||
assert len(fix['artifactChanges']) == 1
|
||||
change = fix['artifactChanges'][0]
|
||||
assert change['artifactLocation']['uri'].endswith('test-fix-it-hint.c')
|
||||
|
||||
@@ -642,6 +642,8 @@ class fixit_hint
|
||||
size_t get_length () const { return m_len; }
|
||||
|
||||
bool insertion_p () const { return m_start == m_next_loc; }
|
||||
bool deletion_p () const { return m_len == 0; }
|
||||
bool replacement_p () const { return m_len > 0 && m_start != m_next_loc; }
|
||||
|
||||
bool ends_with_newline_p () const;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user