diff mbox series

[14/16] diagnostics: SARIF output: add "annotations" property (§3.28.6)

Message ID 20240724221824.585054-15-dmalcolm@redhat.com
State New
Headers show
Series Revamp of JSON/SARIF output | expand

Commit Message

David Malcolm July 24, 2024, 10:18 p.m. UTC
This patch extends our SARIF output so that if a diagnostic has any
labelled source ranges, the "location" object gains an "annotations"
property capturing them (§3.28.6).

For example, given this textual output:

../../src/gcc/testsuite/gcc.dg/bad-binary-ops.c: In function ‘test_2’:
../../src/gcc/testsuite/gcc.dg/bad-binary-ops.c:31:11: error: invalid operands to binary + (have ‘struct s’ and ‘struct t’)
   30 |   return (some_function ()
      |           ~~~~~~~~~~~~~~~~
      |           |
      |           struct s
   31 |           + some_other_function ());
      |           ^ ~~~~~~~~~~~~~~~~~~~~~~
      |             |
      |             struct t

the SARIF output gains this within the result's location[0]:

   "annotations": [{"startLine": 30,
                    "startColumn": 11,
                    "endColumn": 27,
                    "message": {"text": "struct s"}},
                   {"startLine": 31,
                    "startColumn": 13,
                    "endColumn": 35,
                    "message": {"text": "struct t"}}]}]},

gcc/ChangeLog:
	* diagnostic-format-sarif.cc
	(sarif_builder::make_location_object): Add "annotations" property if
	there are any labelled ranges (§3.28.6).
	(selftest::test_make_location_object): Verify annotations are added
	to location_obj.
	* json.h (json::array::size): New.
	(json::array::operator[]): New.
	* selftest-json.cc
	(selftest::expect_json_object_with_array_property): New.
	* selftest-json.h
	(selftest::expect_json_object_with_array_property): New decl.
	(EXPECT_JSON_OBJECT_WITH_ARRAY_PROPERTY): New macro.

gcc/testsuite/ChangeLog:
	* c-c++-common/diagnostic-format-sarif-file-Wbidi-chars.c: Verify
	that we have an "annotations" property for the labelled
	ranges (§3.28.6).

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/diagnostic-format-sarif.cc                | 70 ++++++++++++++++++-
 gcc/json.h                                    |  3 +
 gcc/selftest-json.cc                          | 16 +++++
 gcc/selftest-json.h                           | 14 ++++
 ...diagnostic-format-sarif-file-Wbidi-chars.c |  8 +++
 5 files changed, 110 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/gcc/diagnostic-format-sarif.cc b/gcc/diagnostic-format-sarif.cc
index 775d01f75744..afb29eab5839 100644
--- a/gcc/diagnostic-format-sarif.cc
+++ b/gcc/diagnostic-format-sarif.cc
@@ -345,6 +345,7 @@  public:
    - CWE metadata
    - diagnostic groups (see limitations below)
    - logical locations (e.g. cfun)
+   - labelled ranges (as annotations)
 
    Known limitations:
    - GCC supports one-deep nesting of diagnostics (via auto_diagnostic_group),
@@ -361,7 +362,6 @@  public:
      ("artifact.hashes" property (SARIF v2.1.0 section 3.24.11).
    - doesn't capture the "analysisTarget" property
      (SARIF v2.1.0 section 3.27.13).
-   - doesn't capture labelled ranges
    - doesn't capture -Werror cleanly
    - doesn't capture inlining information (can SARIF handle this?)
    - doesn't capture macro expansion information (can SARIF handle this?).  */
@@ -1210,6 +1210,38 @@  sarif_builder::make_location_object (const rich_location &rich_loc,
   /* "logicalLocations" property (SARIF v2.1.0 section 3.28.4).  */
   set_any_logical_locs_arr (*location_obj, logical_loc);
 
+  /* "annotations" property (SARIF v2.1.0 section 3.28.6).  */
+  {
+    /* Create annotations for any labelled ranges.  */
+    std::unique_ptr<json::array> annotations_arr = nullptr;
+    for (unsigned int i = 0; i < rich_loc.get_num_locations (); i++)
+      {
+	const location_range *range = rich_loc.get_range (i);
+	if (const range_label *label = range->m_label)
+	  {
+	    label_text text = label->get_text (i);
+	    if (text.get ())
+	      {
+		location_t range_loc = rich_loc.get_loc (i);
+		auto region
+		  = maybe_make_region_object (range_loc,
+					      rich_loc.get_column_override ());
+		if (region)
+		  {
+		    if (!annotations_arr)
+		      annotations_arr = ::make_unique<json::array> ();
+		    region->set<sarif_message>
+		      ("message", make_message_object (text.get ()));
+		    annotations_arr->append<sarif_region> (std::move (region));
+		  }
+	      }
+	  }
+      }
+    if (annotations_arr)
+      location_obj->set<json::array> ("annotations",
+				      std::move (annotations_arr));
+  }
+
   /* A flag for hinting that the diagnostic involves issues at the
      level of character encodings (such as homoglyphs, or misleading
      bidirectional control codes), and thus that it will be helpful
@@ -2416,6 +2448,9 @@  test_make_location_object (const line_table_case &case_)
 
   sarif_builder builder (dc, "MAIN_INPUT_FILENAME", true);
 
+  /* These "columns" are byte offsets, whereas later on the columns
+     in the generated SARIF use sarif_builder::get_sarif_column and
+     thus respect tabs, encoding.  */
   const location_t foo
     = make_location (linemap_position_for_column (line_table, 1),
 		     linemap_position_for_column (line_table, 1),
@@ -2480,6 +2515,39 @@  test_make_location_object (const line_table_case &case_)
       }
     }
   }
+  auto annotations
+    = EXPECT_JSON_OBJECT_WITH_ARRAY_PROPERTY (location_obj.get (),
+					      "annotations");
+  ASSERT_EQ (annotations->size (), 3);
+  {
+    {
+      auto a0 = (*annotations)[0];
+      ASSERT_JSON_INT_PROPERTY_EQ (a0, "startLine", 1);
+      ASSERT_JSON_INT_PROPERTY_EQ (a0, "startColumn", 1);
+      ASSERT_JSON_INT_PROPERTY_EQ (a0, "endColumn", 7);
+      auto message
+	= EXPECT_JSON_OBJECT_WITH_OBJECT_PROPERTY (a0, "message");
+      ASSERT_JSON_STRING_PROPERTY_EQ (message, "text", "label0");
+    }
+    {
+      auto a1 = (*annotations)[1];
+      ASSERT_JSON_INT_PROPERTY_EQ (a1, "startLine", 1);
+      ASSERT_JSON_INT_PROPERTY_EQ (a1, "startColumn", 10);
+      ASSERT_JSON_INT_PROPERTY_EQ (a1, "endColumn", 15);
+      auto message
+	= EXPECT_JSON_OBJECT_WITH_OBJECT_PROPERTY (a1, "message");
+      ASSERT_JSON_STRING_PROPERTY_EQ (message, "text", "label1");
+    }
+    {
+      auto a2 = (*annotations)[2];
+      ASSERT_JSON_INT_PROPERTY_EQ (a2, "startLine", 1);
+      ASSERT_JSON_INT_PROPERTY_EQ (a2, "startColumn", 16);
+      ASSERT_JSON_INT_PROPERTY_EQ (a2, "endColumn", 25);
+      auto message
+	= EXPECT_JSON_OBJECT_WITH_OBJECT_PROPERTY (a2, "message");
+      ASSERT_JSON_STRING_PROPERTY_EQ (message, "text", "label2");
+    }
+  }
 }
 
 /* Run all of the selftests within this file.  */
diff --git a/gcc/json.h b/gcc/json.h
index f80a5e82caf3..96721edf5365 100644
--- a/gcc/json.h
+++ b/gcc/json.h
@@ -170,6 +170,9 @@  class array : public value
     append (v.release ());
   }
 
+  size_t size () const { return m_elements.length (); }
+  value *operator[] (size_t i) const { return m_elements[i]; }
+
  private:
   auto_vec<value *> m_elements;
 };
diff --git a/gcc/selftest-json.cc b/gcc/selftest-json.cc
index 86f27cb82999..271e9b441120 100644
--- a/gcc/selftest-json.cc
+++ b/gcc/selftest-json.cc
@@ -96,6 +96,22 @@  expect_json_object_with_object_property (const location &loc,
   return static_cast<const json::object *> (property_value);
 }
 
+/* Assert that VALUE is a non-null json::object that has property
+   PROPERTY_NAME, and that the property value is a non-null JSON array.
+   Return the value of the property as a json::array.
+   Use LOC for any failures.  */
+
+const json::array *
+expect_json_object_with_array_property (const location &loc,
+					const json::value *value,
+					const char *property_name)
+{
+  const json::value *property_value
+    = expect_json_object_with_property (loc, value, property_name);
+  ASSERT_EQ_AT (loc, property_value->get_kind (), json::JSON_ARRAY);
+  return static_cast<const json::array *> (property_value);
+}
+
 /* Assert that VALUE is a non-null json::object that has property
    PROPERTY_NAME, and that the value of that property is a non-null
    JSON string equalling EXPECTED_VALUE.
diff --git a/gcc/selftest-json.h b/gcc/selftest-json.h
index 75a20d519a4c..23b4d18951ca 100644
--- a/gcc/selftest-json.h
+++ b/gcc/selftest-json.h
@@ -77,6 +77,20 @@  expect_json_object_with_object_property (const location &loc,
 					   (JSON_VALUE),		\
 					   (PROPERTY_NAME))
 
+/* Assert that VALUE is a non-null json::object that has property
+   PROPERTY_NAME, and that the property value is a non-null JSON array.
+   Return the value of the property as a json::array.
+   Use LOC for any failures.  */
+
+const json::array *
+expect_json_object_with_array_property (const location &loc,
+					const json::value *value,
+					const char *property_name);
+#define EXPECT_JSON_OBJECT_WITH_ARRAY_PROPERTY(JSON_VALUE, PROPERTY_NAME) \
+  expect_json_object_with_array_property ((SELFTEST_LOCATION),		\
+					  (JSON_VALUE),		\
+					  (PROPERTY_NAME))
+
 /* Assert that VALUE is a non-null json::object that has property
    PROPERTY_NAME, and that the value of that property is a non-null
    JSON string equalling EXPECTED_VALUE.
diff --git a/gcc/testsuite/c-c++-common/diagnostic-format-sarif-file-Wbidi-chars.c b/gcc/testsuite/c-c++-common/diagnostic-format-sarif-file-Wbidi-chars.c
index 8a287d6c8683..f6084ad04a6f 100644
--- a/gcc/testsuite/c-c++-common/diagnostic-format-sarif-file-Wbidi-chars.c
+++ b/gcc/testsuite/c-c++-common/diagnostic-format-sarif-file-Wbidi-chars.c
@@ -29,4 +29,12 @@  int main() {
 
      { dg-final { scan-sarif-file {"rendered": } } }
 
+   Verify that we have an "annotations" property for the
+   labelled ranges (3.28.6).
+     { dg-final { scan-sarif-file {"annotations": } } }
+   and that the annotations capture the labels as messages,
+   using "." in place of awkard characters:
+     { dg-final { scan-sarif-file {"message": ."text": "end of bidirectional context"} } }
+     { dg-final { scan-sarif-file {"message": ."text": "U.202E .RIGHT-TO-LEFT OVERRIDE."} } }
+     { dg-final { scan-sarif-file {"message": ."text": "U.2066 .LEFT-TO-RIGHT ISOLATE."} } }
 */