From 5149ffdd16f46b0dddb60552d162fd772e9a45df Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Wed, 21 Apr 2021 12:01:09 +0200 Subject: [PATCH 1/9] C#: Add extraction error diagnostic query --- .../Diagnostics/DiagnosticExtractionErrors.ql | 18 ++++++++++++++++++ .../DiagnosticExtractorErrors.expected | 0 .../DiagnosticExtractorErrors.qlref | 1 + .../test/library-tests/diagnostics/Program.cs | 17 +++++++++++++++++ 4 files changed, 36 insertions(+) create mode 100644 csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql create mode 100644 csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected create mode 100644 csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.qlref create mode 100644 csharp/ql/test/library-tests/diagnostics/Program.cs diff --git a/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql new file mode 100644 index 000000000000..e2fdf2751a43 --- /dev/null +++ b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql @@ -0,0 +1,18 @@ +/** + * @name Extraction errors + * @description List all errors reported by the extractor. The returned issues are + * limited to those files where there are no compilation errors. This + * indicates a bug or limitation in the extractor, and could lead to + * inaccurate results. + * @kind diagnostic + * @id cs/diagnostics/extraction-errors + */ + +import csharp +import semmle.code.csharp.commons.Diagnostics + +from ExtractorError error +where not exists(CompilerError ce | ce.getLocation().getFile() = error.getLocation().getFile()) +select error, + "Unexpected " + error.getOrigin() + " error: " + error.getText() + " in " + + error.getLocation().getFile(), 3 diff --git a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.qlref b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.qlref new file mode 100644 index 000000000000..7068705cc1be --- /dev/null +++ b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.qlref @@ -0,0 +1 @@ +Diagnostics/DiagnosticExtractionErrors.ql diff --git a/csharp/ql/test/library-tests/diagnostics/Program.cs b/csharp/ql/test/library-tests/diagnostics/Program.cs new file mode 100644 index 000000000000..7f0a7e57d1cd --- /dev/null +++ b/csharp/ql/test/library-tests/diagnostics/Program.cs @@ -0,0 +1,17 @@ +// semmle-extractor-options: --standalone + +using System; + +class Class +{ + static void Main(string[] args) + { + int z = GetParamLength(__arglist(1, 2)); + } + + public static int GetParamLength(__arglist) + { + ArgIterator iterator = new ArgIterator(__arglist); + return iterator.GetRemainingCount(); + } +} From 353d43a03930efd657638ad8a39e11d4eecbdfc2 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Wed, 21 Apr 2021 12:38:55 +0200 Subject: [PATCH 2/9] Log model errors even in standalone extraction --- csharp/extractor/Semmle.Extraction/Context.cs | 22 ++++++++++++++----- .../Diagnostics/DiagnosticExtractionErrors.ql | 9 ++++++-- .../DiagnosticExtractorErrors.expected | 2 ++ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction/Context.cs b/csharp/extractor/Semmle.Extraction/Context.cs index c0580db96a7d..a60d82c6952a 100644 --- a/csharp/extractor/Semmle.Extraction/Context.cs +++ b/csharp/extractor/Semmle.Extraction/Context.cs @@ -376,6 +376,19 @@ private void ExtractionError(Message msg) Extractor.Message(msg); } + private void ExtractionError(InternalError error) + { + ExtractionError(new Message(error.Message, error.EntityText, CreateLocation(error.Location), error.StackTrace, Severity.Error)); + } + + private void ReportError(InternalError error) + { + if (!Extractor.Standalone) + throw error; + + ExtractionError(error); + } + /// /// Signal an error in the program model. /// @@ -383,8 +396,7 @@ private void ExtractionError(Message msg) /// The error message. public void ModelError(SyntaxNode node, string msg) { - if (!Extractor.Standalone) - throw new InternalError(node, msg); + ReportError(new InternalError(node, msg)); } /// @@ -394,8 +406,7 @@ public void ModelError(SyntaxNode node, string msg) /// The error message. public void ModelError(ISymbol symbol, string msg) { - if (!Extractor.Standalone) - throw new InternalError(symbol, msg); + ReportError(new InternalError(symbol, msg)); } /// @@ -404,8 +415,7 @@ public void ModelError(ISymbol symbol, string msg) /// The error message. public void ModelError(string msg) { - if (!Extractor.Standalone) - throw new InternalError(msg); + ReportError(new InternalError(msg)); } /// diff --git a/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql index e2fdf2751a43..91a3c76b2546 100644 --- a/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql +++ b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql @@ -11,8 +11,13 @@ import csharp import semmle.code.csharp.commons.Diagnostics +private string getLocation(ExtractorError error) { + if error.getLocation().getFile().fromSource() + then result = " in " + error.getLocation().getFile() + else result = "" +} + from ExtractorError error where not exists(CompilerError ce | ce.getLocation().getFile() = error.getLocation().getFile()) select error, - "Unexpected " + error.getOrigin() + " error: " + error.getText() + " in " + - error.getLocation().getFile(), 3 + "Unexpected " + error.getOrigin() + " error" + getLocation(error) + ": " + error.getText(), 3 diff --git a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected index e69de29bb2d1..d83e9827256a 100644 --- a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected +++ b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected @@ -0,0 +1,2 @@ +| Program.cs:9:32:9:46 | Unable to resolve target for call. (Compilation error?) | Unexpected C# extractor error in Program.cs: Unable to resolve target for call. (Compilation error?) | 3 | +| file://:0:0:0:0 | Unhandled literal type | Unexpected C# extractor error: Unhandled literal type | 3 | From b05e211e2133ec4eaa910393fa5e8282c8f632a2 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Wed, 21 Apr 2021 13:48:53 +0200 Subject: [PATCH 3/9] Fix failing test --- .../errorrecovery/DiagnosticsAndErrors.expected | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/csharp/ql/test/library-tests/standalone/errorrecovery/DiagnosticsAndErrors.expected b/csharp/ql/test/library-tests/standalone/errorrecovery/DiagnosticsAndErrors.expected index 13bdd30b1c40..917d625c10c3 100644 --- a/csharp/ql/test/library-tests/standalone/errorrecovery/DiagnosticsAndErrors.expected +++ b/csharp/ql/test/library-tests/standalone/errorrecovery/DiagnosticsAndErrors.expected @@ -1,3 +1,13 @@ compilationMessages extractorMessages +| errors.cs:8:1:8:22 | Namespace not found | +| errors.cs:24:31:24:32 | Failed to determine type | +| errors.cs:24:31:24:40 | Failed to determine type | +| errors.cs:24:31:24:40 | Unable to resolve target for call. (Compilation error?) | +| errors.cs:24:38:24:39 | Failed to determine type | +| errors.cs:57:20:57:20 | Failed to determine type | +| errors.cs:93:45:93:45 | Failed to determine type | +| errors.cs:93:45:93:45 | Failed to resolve name | +| errors.cs:94:45:94:45 | Failed to determine type | +| errors.cs:94:45:94:45 | Failed to resolve name | | file://:0:0:0:0 | Extracting default argument value 'object RecordNumber = default' instead of 'object RecordNumber = -1'. The latter is not supported in C#. | From ff9327a03572d4774308dad1f640af9b73ea5736 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Wed, 21 Apr 2021 14:41:53 +0200 Subject: [PATCH 4/9] Add diagnostic query to get correctly extracted files --- .../Diagnostics/DiagnosticNoExtractionErrors.ql | 14 ++++++++++++++ csharp/ql/test/library-tests/diagnostics/A.cs | 4 ++++ .../DiagnosticNoExtractorErrors.expected | 1 + .../diagnostics/DiagnosticNoExtractorErrors.qlref | 1 + 4 files changed, 20 insertions(+) create mode 100644 csharp/ql/src/Diagnostics/DiagnosticNoExtractionErrors.ql create mode 100644 csharp/ql/test/library-tests/diagnostics/A.cs create mode 100644 csharp/ql/test/library-tests/diagnostics/DiagnosticNoExtractorErrors.expected create mode 100644 csharp/ql/test/library-tests/diagnostics/DiagnosticNoExtractorErrors.qlref diff --git a/csharp/ql/src/Diagnostics/DiagnosticNoExtractionErrors.ql b/csharp/ql/src/Diagnostics/DiagnosticNoExtractionErrors.ql new file mode 100644 index 000000000000..11ef3b327fa0 --- /dev/null +++ b/csharp/ql/src/Diagnostics/DiagnosticNoExtractionErrors.ql @@ -0,0 +1,14 @@ +/** + * @name Successfully extracted files + * @description A list of all files in the source code directory that were extracted + * without encountering an extraction error in the file. + * @kind diagnostic + * @id cs/diagnostics/successfully-extracted-files + */ + +import csharp +import semmle.code.csharp.commons.Diagnostics + +from File file +where file.fromSource() and not exists(ExtractorError e | e.getLocation().getFile() = file) +select file, "" diff --git a/csharp/ql/test/library-tests/diagnostics/A.cs b/csharp/ql/test/library-tests/diagnostics/A.cs new file mode 100644 index 000000000000..21a5b7f52616 --- /dev/null +++ b/csharp/ql/test/library-tests/diagnostics/A.cs @@ -0,0 +1,4 @@ +public class A +{ + public void M() { } +} \ No newline at end of file diff --git a/csharp/ql/test/library-tests/diagnostics/DiagnosticNoExtractorErrors.expected b/csharp/ql/test/library-tests/diagnostics/DiagnosticNoExtractorErrors.expected new file mode 100644 index 000000000000..17e4dc54b7dd --- /dev/null +++ b/csharp/ql/test/library-tests/diagnostics/DiagnosticNoExtractorErrors.expected @@ -0,0 +1 @@ +| A.cs:0:0:0:0 | A.cs | | diff --git a/csharp/ql/test/library-tests/diagnostics/DiagnosticNoExtractorErrors.qlref b/csharp/ql/test/library-tests/diagnostics/DiagnosticNoExtractorErrors.qlref new file mode 100644 index 000000000000..7994e050699f --- /dev/null +++ b/csharp/ql/test/library-tests/diagnostics/DiagnosticNoExtractorErrors.qlref @@ -0,0 +1 @@ +Diagnostics/DiagnosticNoExtractionErrors.ql From 64354bbfaac18b9845033af46e01c9cf86d431db Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 22 Apr 2021 09:23:59 +0200 Subject: [PATCH 5/9] Fix test results after rebase --- .../library-tests/diagnostics/DiagnosticExtractorErrors.expected | 1 - 1 file changed, 1 deletion(-) diff --git a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected index d83e9827256a..f1f4d9750a8d 100644 --- a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected +++ b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected @@ -1,2 +1 @@ | Program.cs:9:32:9:46 | Unable to resolve target for call. (Compilation error?) | Unexpected C# extractor error in Program.cs: Unable to resolve target for call. (Compilation error?) | 3 | -| file://:0:0:0:0 | Unhandled literal type | Unexpected C# extractor error: Unhandled literal type | 3 | From 1a708affbfb30b137746359ba0dcc555cc86fbcc Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 22 Apr 2021 10:08:33 +0200 Subject: [PATCH 6/9] Include compilation errors in diagnostic check --- .../Diagnostics/DiagnosticExtractionErrors.ql | 56 +++++++++++++++---- .../DiagnosticNoExtractionErrors.ql | 7 ++- .../DiagnosticExtractorErrors.expected | 2 +- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql index 91a3c76b2546..f19fcc6416ac 100644 --- a/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql +++ b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql @@ -1,9 +1,7 @@ /** * @name Extraction errors - * @description List all errors reported by the extractor. The returned issues are - * limited to those files where there are no compilation errors. This - * indicates a bug or limitation in the extractor, and could lead to - * inaccurate results. + * @description List all errors reported by the extractor or the compiler. Extractor errors are + * limited to those files where there are no compilation errors. * @kind diagnostic * @id cs/diagnostics/extraction-errors */ @@ -11,13 +9,47 @@ import csharp import semmle.code.csharp.commons.Diagnostics -private string getLocation(ExtractorError error) { - if error.getLocation().getFile().fromSource() - then result = " in " + error.getLocation().getFile() - else result = "" +private newtype TDiagnosticError = + TCompilerError(CompilerError c) or + TExtractorError(ExtractorError e) + +abstract private class DiagnosticError extends TDiagnosticError { + string getMessage() { none() } + + string toString() { none() } + + string getLocation(Location l) { + if l.getFile().fromSource() then result = " in " + l.getFile() else result = "" + } +} + +private class DiagnosticCompilerError extends DiagnosticError { + CompilerError c; + + DiagnosticCompilerError() { this = TCompilerError(c) } + + override string getMessage() { + result = "Compiler error" + getLocation(c.getLocation()) + ": " + c.getMessage() + } + + override string toString() { result = c.toString() } +} + +private class DiagnosticExtractorError extends DiagnosticError { + ExtractorError e; + + DiagnosticExtractorError() { + this = TExtractorError(e) and + not exists(CompilerError ce | ce.getLocation().getFile() = e.getLocation().getFile()) + } + + override string getMessage() { + result = + "Unexpected " + e.getOrigin() + " error" + getLocation(e.getLocation()) + ": " + e.getText() + } + + override string toString() { result = e.toString() } } -from ExtractorError error -where not exists(CompilerError ce | ce.getLocation().getFile() = error.getLocation().getFile()) -select error, - "Unexpected " + error.getOrigin() + " error" + getLocation(error) + ": " + error.getText(), 3 +from DiagnosticError error +select error.getMessage(), 3 diff --git a/csharp/ql/src/Diagnostics/DiagnosticNoExtractionErrors.ql b/csharp/ql/src/Diagnostics/DiagnosticNoExtractionErrors.ql index 11ef3b327fa0..cb86e293efcb 100644 --- a/csharp/ql/src/Diagnostics/DiagnosticNoExtractionErrors.ql +++ b/csharp/ql/src/Diagnostics/DiagnosticNoExtractionErrors.ql @@ -1,7 +1,7 @@ /** * @name Successfully extracted files * @description A list of all files in the source code directory that were extracted - * without encountering an extraction error in the file. + * without encountering an extraction or compiler error in the file. * @kind diagnostic * @id cs/diagnostics/successfully-extracted-files */ @@ -10,5 +10,8 @@ import csharp import semmle.code.csharp.commons.Diagnostics from File file -where file.fromSource() and not exists(ExtractorError e | e.getLocation().getFile() = file) +where + file.fromSource() and + not exists(ExtractorError e | e.getLocation().getFile() = file) and + not exists(CompilerError e | e.getLocation().getFile() = file) select file, "" diff --git a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected index f1f4d9750a8d..e47e78cdce94 100644 --- a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected +++ b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected @@ -1 +1 @@ -| Program.cs:9:32:9:46 | Unable to resolve target for call. (Compilation error?) | Unexpected C# extractor error in Program.cs: Unable to resolve target for call. (Compilation error?) | 3 | +| Unexpected C# extractor error in Program.cs: Unable to resolve target for call. (Compilation error?) | 3 | From e3f10c0e32f0d5659c594cf53fc19042d1fa58d4 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 23 Apr 2021 13:37:42 +0200 Subject: [PATCH 7/9] Cleanup DiagnosticError classes --- .../Diagnostics/DiagnosticExtractionErrors.ql | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql index f19fcc6416ac..91ed4a7b8838 100644 --- a/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql +++ b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql @@ -14,12 +14,16 @@ private newtype TDiagnosticError = TExtractorError(ExtractorError e) abstract private class DiagnosticError extends TDiagnosticError { - string getMessage() { none() } + abstract string getMessage(); - string toString() { none() } + abstract string toString(); - string getLocation(Location l) { - if l.getFile().fromSource() then result = " in " + l.getFile() else result = "" + abstract Location getLocation(); + + string getLocationMessage() { + if getLocation().getFile().fromSource() + then result = " in " + getLocation().getFile() + else result = "" } } @@ -29,10 +33,12 @@ private class DiagnosticCompilerError extends DiagnosticError { DiagnosticCompilerError() { this = TCompilerError(c) } override string getMessage() { - result = "Compiler error" + getLocation(c.getLocation()) + ": " + c.getMessage() + result = "Compiler error" + this.getLocationMessage() + ": " + c.getMessage() } override string toString() { result = c.toString() } + + override Location getLocation() { result = c.getLocation() } } private class DiagnosticExtractorError extends DiagnosticError { @@ -45,10 +51,12 @@ private class DiagnosticExtractorError extends DiagnosticError { override string getMessage() { result = - "Unexpected " + e.getOrigin() + " error" + getLocation(e.getLocation()) + ": " + e.getText() + "Unexpected " + e.getOrigin() + " error" + this.getLocationMessage() + ": " + e.getText() } override string toString() { result = e.toString() } + + override Location getLocation() { result = e.getLocation() } } from DiagnosticError error From b4bd7af9c86a5f89095ee033574753bf46b8dae2 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 23 Apr 2021 13:40:12 +0200 Subject: [PATCH 8/9] Add change note --- csharp/change-notes/2021-04-23-model-error-extraction.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 csharp/change-notes/2021-04-23-model-error-extraction.md diff --git a/csharp/change-notes/2021-04-23-model-error-extraction.md b/csharp/change-notes/2021-04-23-model-error-extraction.md new file mode 100644 index 000000000000..7ac878442984 --- /dev/null +++ b/csharp/change-notes/2021-04-23-model-error-extraction.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Program model errors are now extracted in standalone extraction. \ No newline at end of file From 51e08d494093b230eafb479782a9a2d42fd19cd2 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Tue, 27 Apr 2021 15:43:19 +0200 Subject: [PATCH 9/9] Fix error severity --- csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql | 2 +- .../diagnostics/DiagnosticExtractorErrors.expected | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql index 91ed4a7b8838..23943d8491fb 100644 --- a/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql +++ b/csharp/ql/src/Diagnostics/DiagnosticExtractionErrors.ql @@ -60,4 +60,4 @@ private class DiagnosticExtractorError extends DiagnosticError { } from DiagnosticError error -select error.getMessage(), 3 +select error.getMessage(), 2 diff --git a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected index e47e78cdce94..e4ae5ec1ef29 100644 --- a/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected +++ b/csharp/ql/test/library-tests/diagnostics/DiagnosticExtractorErrors.expected @@ -1 +1 @@ -| Unexpected C# extractor error in Program.cs: Unable to resolve target for call. (Compilation error?) | 3 | +| Unexpected C# extractor error in Program.cs: Unable to resolve target for call. (Compilation error?) | 2 |