Merge pull request #176 from github/diagnostics-entries

Start writing diagnostics to the DB, and some basic summary/diagnostics queries

extractor/src/extractor.rs

@@ -305,22 +305,58 @@ struct Visitor<'a> {
 }
 
 impl Visitor<'_> {
+    fn record_parse_error(
+        &mut self,
+        error_message: String,
+        full_error_message: String,
+        loc: Label,
+    ) {
+        error!("{}", full_error_message);
+        let id = self.trap_writer.fresh_id();
+        self.trap_writer.add_tuple(
+            "diagnostics",
+            vec![
+                Arg::Label(id),
+                Arg::Int(40), // severity 40 = error
+                Arg::String("parse_error".to_string()),
+                Arg::String(error_message),
+                Arg::String(full_error_message),
+                Arg::Label(loc),
+            ],
+        );
+    }
+
+    fn record_parse_error_for_node(
+        &mut self,
+        error_message: String,
+        full_error_message: String,
+        node: Node,
+    ) {
+        let (start_line, start_column, end_line, end_column) = location_for(&self.source, node);
+        let loc = self.trap_writer.location(
+            self.file_label,
+            start_line,
+            start_column,
+            end_line,
+            end_column,
+        );
+        self.record_parse_error(error_message, full_error_message, loc);
+    }
+
     fn enter_node(&mut self, node: Node) -> bool {
-        if node.is_error() {
-            error!(
-                "{}:{}: parse error",
-                &self.path,
-                node.start_position().row + 1
-            );
-            return false;
-        }
-        if node.is_missing() {
-            error!(
-                "{}:{}: parse error: expecting '{}'",
+        if node.is_error() || node.is_missing() {
+            let error_message = if node.is_missing() {
+                format!("parse error: expecting '{}'", node.kind())
+            } else {
+                "parse error".to_string()
+            };
+            let full_error_message = format!(
+                "{}:{}: {}",
                 &self.path,
                 node.start_position().row + 1,
-                node.kind()
+                error_message
             );
+            self.record_parse_error_for_node(error_message, full_error_message, node);
             return false;
         }
 
@@ -405,12 +441,15 @@ impl Visitor<'_> {
                 }
             }
             _ => {
-                error!(
-                    "{}:{}: unknown table type: '{}'",
+                let error_message = format!("unknown table type: '{}'", node.kind());
+                let full_error_message = format!(
+                    "{}:{}: {}",
                     &self.path,
                     node.start_position().row + 1,
-                    node.kind()
+                    error_message
                 );
+                self.record_parse_error(error_message, full_error_message, loc);
+
                 valid = false;
             }
         }
@@ -456,26 +495,36 @@ impl Visitor<'_> {
                         values.push(Arg::Label(child_node.label));
                     }
                 } else if field.name.is_some() {
-                    error!(
-                        "{}:{}: type mismatch for field {}::{} with type {:?} != {:?}",
-                        &self.path,
-                        node.start_position().row + 1,
+                    let error_message = format!(
+                        "type mismatch for field {}::{} with type {:?} != {:?}",
                         node.kind(),
                         child_node.field_name.unwrap_or("child"),
                         child_node.type_name,
                         field.type_info
-                    )
+                    );
+                    let full_error_message = format!(
+                        "{}:{}: {}",
+                        &self.path,
+                        node.start_position().row + 1,
+                        error_message
+                    );
+                    self.record_parse_error_for_node(error_message, full_error_message, *node);
                 }
             } else {
                 if child_node.field_name.is_some() || child_node.type_name.named {
-                    error!(
-                        "{}:{}: value for unknown field: {}::{} and type {:?}",
-                        &self.path,
-                        node.start_position().row + 1,
+                    let error_message = format!(
+                        "value for unknown field: {}::{} and type {:?}",
                         node.kind(),
                         &child_node.field_name.unwrap_or("child"),
                         &child_node.type_name
                     );
+                    let full_error_message = format!(
+                        "{}:{}: {}",
+                        &self.path,
+                        node.start_position().row + 1,
+                        error_message
+                    );
+                    self.record_parse_error_for_node(error_message, full_error_message, *node);
                 }
             }
         }
@@ -489,10 +538,8 @@ impl Visitor<'_> {
                         args.push(child_values.first().unwrap().clone());
                     } else {
                         is_valid = false;
-                        error!(
-                            "{}:{}: {} for field: {}::{}",
-                            &self.path,
-                            node.start_position().row + 1,
+                        let error_message = format!(
+                            "{} for field: {}::{}",
                             if child_values.is_empty() {
                                 "missing value"
                             } else {
@@ -500,7 +547,14 @@ impl Visitor<'_> {
                             },
                             node.kind(),
                             column_name
-                        )
+                        );
+                        let full_error_message = format!(
+                            "{}:{}: {}",
+                            &self.path,
+                            node.start_position().row + 1,
+                            error_message
+                        );
+                        self.record_parse_error_for_node(error_message, full_error_message, *node);
                     }
                 }
                 Storage::Table {

generator/src/main.rs

@@ -248,6 +248,11 @@ fn convert_nodes<'a>(nodes: &'a node_types::NodeTypeMap) -> Vec<dbscheme::Entry<
     entries.push(dbscheme::Entry::Table(token_table));
     entries.push(dbscheme::Entry::Case(token_case));
 
+    // Add the diagnostics table
+    let (diagnostics_case, diagnostics_table) = create_diagnostics();
+    entries.push(dbscheme::Entry::Table(diagnostics_table));
+    entries.push(dbscheme::Entry::Case(diagnostics_case));
+
     // Create a union of all database types.
     entries.push(dbscheme::Entry::Union(dbscheme::Union {
         name: "ast_node",
@@ -590,6 +595,69 @@ fn create_source_location_prefix_table<'a>() -> dbscheme::Entry<'a> {
     })
 }
 
+fn create_diagnostics<'a>() -> (dbscheme::Case<'a>, dbscheme::Table<'a>) {
+    let table = dbscheme::Table {
+        name: "diagnostics",
+        keysets: None,
+        columns: vec![
+            dbscheme::Column {
+                unique: true,
+                db_type: dbscheme::DbColumnType::Int,
+                name: "id",
+                ql_type: ql::Type::AtType("diagnostic"),
+                ql_type_is_ref: false,
+            },
+            dbscheme::Column {
+                unique: false,
+                db_type: dbscheme::DbColumnType::Int,
+                name: "severity",
+                ql_type: ql::Type::Int,
+                ql_type_is_ref: true,
+            },
+            dbscheme::Column {
+                unique: false,
+                db_type: dbscheme::DbColumnType::String,
+                name: "error_tag",
+                ql_type: ql::Type::String,
+                ql_type_is_ref: true,
+            },
+            dbscheme::Column {
+                unique: false,
+                db_type: dbscheme::DbColumnType::String,
+                name: "error_message",
+                ql_type: ql::Type::String,
+                ql_type_is_ref: true,
+            },
+            dbscheme::Column {
+                unique: false,
+                db_type: dbscheme::DbColumnType::String,
+                name: "full_error_message",
+                ql_type: ql::Type::String,
+                ql_type_is_ref: true,
+            },
+            dbscheme::Column {
+                unique: false,
+                db_type: dbscheme::DbColumnType::Int,
+                name: "location",
+                ql_type: ql::Type::AtType("location_default"),
+                ql_type_is_ref: true,
+            },
+        ],
+    };
+    let severities: Vec<(usize, &str)> = vec![
+        (10, "diagnostic_debug"),
+        (20, "diagnostic_info"),
+        (30, "diagnostic_warning"),
+        (40, "diagnostic_error"),
+    ];
+    let case = dbscheme::Case {
+        name: "diagnostic",
+        column: "severity",
+        branches: severities,
+    };
+    (case, table)
+}
+
 fn main() {
     tracing_subscriber::fmt()
         .with_target(false)

ql/src/codeql_ruby/Diagnostics.qll

@@ -0,0 +1,52 @@
+private import codeql.Locations
+
+/** A diagnostic emitted during extraction, such as a parse error */
+class Diagnostic extends @diagnostic {
+  int severity;
+  string tag;
+  string message;
+  string fullMessage;
+  Location location;
+
+  Diagnostic() { diagnostics(this, severity, tag, message, fullMessage, location) }
+
+  /**
+   * Gets the numerical severity level associated with this diagnostic.
+   */
+  int getSeverity() { result = severity }
+
+  /** Gets a string representation of the severity of this diagnostic. */
+  string getSeverityText() {
+    severity = 10 and result = "Debug"
+    or
+    severity = 20 and result = "Info"
+    or
+    severity = 30 and result = "Warning"
+    or
+    severity = 40 and result = "Error"
+  }
+
+  /** Gets the error code associated with this diagnostic, e.g. parse_error. */
+  string getTag() { result = tag }
+
+  /**
+   * Gets the error message text associated with this diagnostic.
+   */
+  string getMessage() { result = message }
+
+  /**
+   * Gets the full error message text associated with this diagnostic.
+   */
+  string getFullMessage() { result = fullMessage }
+
+  /** Gets the source location of this diagnostic. */
+  Location getLocation() { result = location }
+
+  /** Gets a textual representation of this diagnostic. */
+  string toString() { result = this.getMessage() }
+}
+
+/** A diagnostic relating to a particular error in extracting a file. */
+class ExtractionError extends Diagnostic, @diagnostic_error {
+  ExtractionError() { this.getTag() = "parse_error" }
+}

ql/src/queries/diagnostics/ExtractionErrors.ql

@@ -0,0 +1,18 @@
+/**
+ * @name Extraction errors
+ * @description List all extraction errors for files in the source code directory.
+ * @kind diagnostic
+ * @id rb/diagnostics/extraction-errors
+ */
+
+import ruby
+import codeql_ruby.Diagnostics
+
+/** Gets the SARIF severity to associate an error. */
+int getSeverity() { result = 2 }
+
+from ExtractionError error, File f
+where
+  f = error.getLocation().getFile() and
+  exists(f.getRelativePath())
+select error, "Extraction failed in " + f + " with error " + error.getMessage(), getSeverity()

ql/src/queries/diagnostics/SuccessfullyExtractedFiles.ql

@@ -0,0 +1,16 @@
+/**
+ * @name Successfully extracted files
+ * @description Lists all files in the source code directory that were extracted
+ *   without encountering an error in the file.
+ * @kind diagnostic
+ * @id rb/diagnostics/successfully-extracted-files
+ */
+
+import ruby
+import codeql_ruby.Diagnostics
+
+from File f
+where
+  not exists(ExtractionError e | e.getLocation().getFile() = f) and
+  exists(f.getRelativePath())
+select f, ""

ql/src/queries/summary/NumberOfFilesExtractedWithErrors.ql

@@ -0,0 +1,15 @@
+/**
+ * @id rb/summary/number-of-files-extracted-with-errors
+ * @name Total number of files that were extracted with errors
+ * @description The total number of Ruby code files that we extracted, but where
+ *  at least one extraction error occurred in the process.
+ * @kind metric
+ * @tags summary
+ */
+
+import ruby
+import codeql_ruby.Diagnostics
+
+select count(File f |
+    exists(ExtractionError e | e.getLocation().getFile() = f) and exists(f.getRelativePath())
+  )

ql/src/queries/summary/NumberOfSuccessfullyExtractedFiles.ql

@@ -0,0 +1,15 @@
+/**
+ * @id rb/summary/number-of-successfully-extracted-files
+ * @name Total number of files that were extracted without error
+ * @description The total number of Ruby code files that we extracted without
+ *   encountering any extraction errors
+ * @kind metric
+ * @tags summary
+ */
+
+import ruby
+import codeql_ruby.Diagnostics
+
+select count(File f |
+    not exists(ExtractionError e | e.getLocation().getFile() = f) and exists(f.getRelativePath())
+  )

ql/src/ruby.dbscheme

@@ -1237,6 +1237,23 @@ case @token.kind of
 ;
 
 
+diagnostics(
+  unique int id: @diagnostic,
+  int severity: int ref,
+  string error_tag: string ref,
+  string error_message: string ref,
+  string full_error_message: string ref,
+  int location: @location_default ref
+);
+
+case @diagnostic.severity of
+  10 = @diagnostic_debug
+| 20 = @diagnostic_info
+| 30 = @diagnostic_warning
+| 40 = @diagnostic_error
+;
+
+
 @ast_node = @alias | @argument_list | @array | @assignment | @bare_string | @bare_symbol | @begin | @begin_block | @binary | @block | @block_argument | @block_parameter | @block_parameters | @break | @call | @case__ | @chained_string | @class | @conditional | @delimited_symbol | @destructured_left_assignment | @destructured_parameter | @do | @do_block | @element_reference | @else | @elsif | @end_block | @ensure | @exception_variable | @exceptions | @for | @hash | @hash_splat_argument | @hash_splat_parameter | @heredoc_body | @if | @if_modifier | @in | @interpolation | @keyword_parameter | @lambda | @lambda_parameters | @left_assignment_list | @method | @method_parameters | @module | @next | @operator_assignment | @optional_parameter | @pair | @parenthesized_statements | @pattern | @program | @range | @rational | @redo | @regex | @rescue | @rescue_modifier | @rest_assignment | @retry | @return | @right_assignment_list | @scope_resolution | @setter | @singleton_class | @singleton_method | @splat_argument | @splat_parameter | @string__ | @string_array | @subshell | @superclass | @symbol_array | @then | @token | @unary | @undef | @unless | @unless_modifier | @until | @until_modifier | @when | @while | @while_modifier | @yield
 
 @ast_node_parent = @ast_node | @file

ql/test/query-tests/diagnostics/ExtractionErrors.expected

@@ -0,0 +1,5 @@
+| src/not_ruby.rb:5:25:5:26 | parse error | Extraction failed in src/not_ruby.rb with error parse error | 2 |
+| src/unsupported_feature.rb:2:18:2:20 | parse error | Extraction failed in src/unsupported_feature.rb with error parse error | 2 |
+| src/unsupported_feature.rb:3:13:3:15 | parse error | Extraction failed in src/unsupported_feature.rb with error parse error | 2 |
+| src/unsupported_feature.rb:6:15:6:17 | parse error | Extraction failed in src/unsupported_feature.rb with error parse error | 2 |
+| src/unsupported_feature.rb:7:20:7:22 | parse error | Extraction failed in src/unsupported_feature.rb with error parse error | 2 |

ql/test/query-tests/diagnostics/ExtractionErrors.qlref

@@ -0,0 +1 @@
+queries/diagnostics/ExtractionErrors.ql

ql/test/query-tests/diagnostics/NumberOfFilesExtractedWithErrors.expected

@@ -0,0 +1 @@
+| 2 |

ql/test/query-tests/diagnostics/NumberOfFilesExtractedWithErrors.qlref

@@ -0,0 +1 @@
+queries/summary/NumberOfFilesExtractedWithErrors.ql

ql/test/query-tests/diagnostics/NumberOfSuccessfullyExtractedFiles.expected

@@ -0,0 +1 @@
+| 2 |

ql/test/query-tests/diagnostics/NumberOfSuccessfullyExtractedFiles.qlref

@@ -0,0 +1 @@
+queries/summary/NumberOfSuccessfullyExtractedFiles.ql

ql/test/query-tests/diagnostics/SuccessfullyExtractedFiles.expected

@@ -0,0 +1,2 @@
+| src/foo.rb:0:0:0:0 | src/foo.rb |  |
+| src/vendor/cache/lib.rb:0:0:0:0 | src/vendor/cache/lib.rb |  |

ql/test/query-tests/diagnostics/SuccessfullyExtractedFiles.qlref

@@ -0,0 +1 @@
+queries/diagnostics/SuccessfullyExtractedFiles.ql

ql/test/query-tests/diagnostics/src/foo.rb

@@ -0,0 +1,9 @@
+# comment
+
+def hello
+  p "hello world"
+end
+
+# another one
+
+hello

ql/test/query-tests/diagnostics/src/not_ruby.rb

@@ -0,0 +1,5 @@
+#!/bin/bash
+
+# This is a bash script
+export FOO="$(whereis ls)"
+exec "$FOO" "$(dirname "$0")"

ql/test/query-tests/diagnostics/src/unsupported_feature.rb

@@ -0,0 +1,17 @@
+class Foo
+  def initialize(...)
+    do_init(...)
+  end
+
+  def do_init(...)
+    really_do_init(...)
+  end
+
+  def really_do_init(bar, baz:, &block)
+    puts bar
+    puts baz
+    block.call
+  end
+end
+
+Foo.new("hello", baz: "world") { || puts "!" }

ql/test/query-tests/diagnostics/src/vendor/cache/lib.rb

@@ -0,0 +1,9 @@
+# comment
+
+def hello
+  p "hello lib"
+end
+
+# another one
+
+hello

upgrades/8725deeb2fa6627c45235f18b7c121c35498dac7/upgrade.properties

@@ -0,0 +1,2 @@
+description: Create an empty diagnostics table
+compatibility: backwards