From: David Malcolm Date: Tue, 23 Sep 2025 20:38:37 +0000 (-0400) Subject: sarif output: add descriptions to fix-it hints (§3.55.2) [PR121986] X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0d5af6a757051708d2dfa510b2ad84f083d7eddc;p=thirdparty%2Fgcc.git 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 --- diff --git a/gcc/diagnostics/sarif-sink.cc b/gcc/diagnostics/sarif-sink.cc index 144040e9dfb..8bd026901c6 100644 --- a/gcc/diagnostics/sarif-sink.cc +++ b/gcc/diagnostics/sarif-sink.cc @@ -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 + make_message_describing_fix_it_hint (const fixit_hint &hint) const; std::unique_ptr make_fix_object (const rich_location &rich_loc); std::unique_ptr @@ -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_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 @@ -3762,6 +3824,18 @@ sarif_builder::make_fix_object (const rich_location &richloc) fix_obj->set ("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 ("description", + std::move (desc_msg)); + } + return fix_obj; } diff --git a/gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.c b/gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.c new file mode 100644 index 00000000000..bcbb267b567 --- /dev/null +++ b/gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.c @@ -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" } } */ diff --git a/gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.py b/gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.py new file mode 100644 index 00000000000..d7f54d24826 --- /dev/null +++ b/gcc/testsuite/gcc.dg/sarif-output/extra-semicolon.py @@ -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'] == '' diff --git a/gcc/testsuite/gcc.dg/sarif-output/missing-semicolon.py b/gcc/testsuite/gcc.dg/sarif-output/missing-semicolon.py index a0f848387a2..374d2ec0cfa 100644 --- a/gcc/testsuite/gcc.dg/sarif-output/missing-semicolon.py +++ b/gcc/testsuite/gcc.dg/sarif-output/missing-semicolon.py @@ -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] diff --git a/gcc/testsuite/libgdiagnostics.dg/test-fix-it-hint-c.py b/gcc/testsuite/libgdiagnostics.dg/test-fix-it-hint-c.py index f3dc71c27b5..cf1466e6983 100644 --- a/gcc/testsuite/libgdiagnostics.dg/test-fix-it-hint-c.py +++ b/gcc/testsuite/libgdiagnostics.dg/test-fix-it-hint-c.py @@ -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') diff --git a/libcpp/include/rich-location.h b/libcpp/include/rich-location.h index c74e80e9f61..cc9547d3ab7 100644 --- a/libcpp/include/rich-location.h +++ b/libcpp/include/rich-location.h @@ -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;