From c6147b061bd3f921004014636f38dc7f2e640751 Mon Sep 17 00:00:00 2001 From: Delma Date: Sat, 14 Sep 2019 20:49:53 +0300 Subject: [PATCH 1/4] Started fixing lint message parsing by changing output format json and using jackson to parse that --- pom.xml | 4 +- tmc-langs-rust/pom.xml | 8 ++- .../cs/tmc/langs/rust/CargoPlugin.java | 2 +- .../langs/rust/CargoStudentFilePolicy.java | 2 +- .../cs/tmc/langs/rust/LinterResultParser.java | 69 ++++++++++--------- 5 files changed, 48 insertions(+), 37 deletions(-) diff --git a/pom.xml b/pom.xml index 7a1556f0b..6ab610c14 100644 --- a/pom.xml +++ b/pom.xml @@ -229,10 +229,10 @@ build-tools tmc-langs-framework tmc-langs-java - tmc-langs-make + tmc-langs-python3 tmc-langs-notests - + tmc-langs-rust tmc-langs-qmake diff --git a/tmc-langs-rust/pom.xml b/tmc-langs-rust/pom.xml index 784bcec21..7778732a6 100644 --- a/tmc-langs-rust/pom.xml +++ b/tmc-langs-rust/pom.xml @@ -3,6 +3,12 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 + + fi.helsinki.cs.tmc + tmc-langs + 0.7.15-SNAPSHOT + + UTF-8 ../target @@ -29,7 +35,7 @@ fi.helsinki.cs.tmc tmc-langs-framework - 0.1.3-SNAPSHOT + ${project.version} jar diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java index 760e9adcd..b8e1d8bdb 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java @@ -64,7 +64,7 @@ protected StudentFilePolicy getStudentFilePolicy(Path projectPath) { @Override public ValidationResult checkCodeStyle(Path path, Locale locale) { if (run(new String[] {"cargo", "clean"}, path).isPresent()) { - String[] command = {"cargo", "rustc", "--", "--forbid", "warnings"}; + String[] command = {"cargo", "--message-format", "json" , "rustc", "--", "--forbid", "warnings"}; log.info("Building for lints with command {}", Arrays.deepToString(command)); Optional result = run(command, path); if (result.isPresent()) { diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoStudentFilePolicy.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoStudentFilePolicy.java index 06b1f9a87..f19fb5d7f 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoStudentFilePolicy.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoStudentFilePolicy.java @@ -20,7 +20,7 @@ public CargoStudentFilePolicy(Path configFileParent) { * decision to move them is made by {@link ConfigurableStudentFilePolicy}. */ @Override - public boolean isStudentSourceFile(Path path) { + public boolean isStudentSourceFile(Path path, Path projectRootPath) { return !path.endsWith(Constants.CARGO_TOML) && path.startsWith(Constants.SOURCE); } diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java index 40ae046d4..03a2f93de 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java @@ -4,52 +4,57 @@ import fi.helsinki.cs.tmc.langs.abstraction.ValidationResult; import fi.helsinki.cs.tmc.langs.utils.ProcessResult; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + import java.io.File; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collector; +import java.util.stream.Collectors; public class LinterResultParser { - //src\lib.rs:3:9: - private static final String START_INFO - = "(?.+):(?\\d+):(?\\d+):"; - //3:14 - private static final String END_INFO = "(?\\d+):(?\\d+)"; - //error: variable does not need to be mutable, #[forbid(unused_mut)] on by default - private static final Pattern ERROR_START - = Pattern.compile(START_INFO + " " + END_INFO + " error: (?.*), .*"); - //src\lib.rs:3 let mut x = a * b; - private static final Pattern ERROR_CONTINUE - = Pattern.compile("(?(?.+):(?\\d+) )(?.*)"); - // ^~~~~ - private static final Pattern ARROW = Pattern.compile("(? *)\\^(?~*)"); + + private static final ObjectMapper mapper = new ObjectMapper(); + /** * Parses given process result to a validation result. */ public ValidationResult parse(ProcessResult processResult) { String output = processResult.errorOutput; String[] lines = output.split("\\r?\\n"); - LinterResult result = new LinterResult(); - List code = new ArrayList<>(); - for (String line : lines) { - Matcher matcher = ERROR_START.matcher(line); - if (matcher.matches()) { - code = new ArrayList<>(); - addLintError(matcher, code, result); - } else { - matcher = ERROR_CONTINUE.matcher(line); - if (matcher.matches()) { - code.add(line); - } else { - matcher = ARROW.matcher(line); - if (!matcher.matches()) { - break; - } - } - } - } + Arrays.stream(lines) + .map((line) -> mapper.readTree(line)) + .filter((parseTree) -> parseTree.get("reason").asText().equals("compiler-message")) + .map((parseTree) -> { + + return null; + }); + + + // LinterResult result = new LinterResult(); + // List code = new ArrayList<>(); + // for (String line : lines) { + // Matcher matcher = ERROR_START.matcher(line); + // if (matcher.matches()) { + // code = new ArrayList<>(); + // addLintError(matcher, code, result); + // } else { + // matcher = ERROR_CONTINUE.matcher(line); + // if (matcher.matches()) { + // code.add(line); + // } else { + // matcher = ARROW.matcher(line); + // if (!matcher.matches()) { + // break; + // } + // } + // } + // } return result; } From 398103f09fdba023c8977457e81683b8cdff9ba8 Mon Sep 17 00:00:00 2001 From: Delma Date: Wed, 2 Oct 2019 21:33:45 +0300 Subject: [PATCH 2/4] What have we done --- .../helsinki/cs/tmc/langs/rust/LintError.java | 10 +- .../cs/tmc/langs/rust/LinterResultParser.java | 22 ++- .../cs/tmc/langs/rust/error_example.json | 144 ++++++++++++++ .../cs/tmc/langs/rust/warnings_example.json | 186 ++++++++++++++++++ 4 files changed, 352 insertions(+), 10 deletions(-) create mode 100644 tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/error_example.json create mode 100644 tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/warnings_example.json diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LintError.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LintError.java index 9ae30832f..b8a8efd7a 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LintError.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LintError.java @@ -9,7 +9,7 @@ public class LintError implements ValidationError { private final List code; private final int startLine; private final int startColumn; - private final String description; + private final String message; private final String file; private final int endLine; private final int endColumn; @@ -18,7 +18,7 @@ public class LintError implements ValidationError { * Creates new error that failing lints generate. * * @param file file that the error is in - * @param description describes what is wrong + * @param message describes what is wrong * @param code list of code lines the error is in * @param startLine from what line error start * @param startColumn from what column in that line error starts @@ -27,7 +27,7 @@ public class LintError implements ValidationError { */ public LintError( String file, - String description, + String message, List code, int startLine, int startColumn, @@ -35,7 +35,7 @@ public LintError( int endColumn) { this.code = code; this.file = file; - this.description = description; + this.message = message; this.startLine = startLine; this.startColumn = startColumn; this.endLine = endLine; @@ -62,7 +62,7 @@ public int getEndLine() { @Override public String getMessage() { - return description; + return message; } @Override diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java index 03a2f93de..1dc9648e8 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java @@ -11,11 +11,13 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collector; import java.util.stream.Collectors; +import java.util.stream.Stream; public class LinterResultParser { @@ -27,13 +29,18 @@ public class LinterResultParser { public ValidationResult parse(ProcessResult processResult) { String output = processResult.errorOutput; String[] lines = output.split("\\r?\\n"); - Arrays.stream(lines) + List errors = Arrays.stream(lines) .map((line) -> mapper.readTree(line)) .filter((parseTree) -> parseTree.get("reason").asText().equals("compiler-message")) - .map((parseTree) -> { - - return null; - }); + .flatMap((parseTree) -> { + for (JsonNode node : parseTree.get("message").get("spans")) { + if (node.get("is_primary").asBoolean()) { + return Stream.of(createLintError(node, parseTree.get("rendered").asText())); + } + } + return Stream.empty(); + }) + .collect(Collectors.toList()); // LinterResult result = new LinterResult(); @@ -58,6 +65,11 @@ public ValidationResult parse(ProcessResult processResult) { return result; } + private LintError createLintError(JsonNode node, String message) { + String file = node.get(fieldName) + return new LintError(file, message, code, startLine, startColumn, endLine, endColumn); + } + private void addLintError(Matcher matcher, List code, LinterResult result) { String fileName = matcher.group("file"); LintError current = new LintError( diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/error_example.json b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/error_example.json new file mode 100644 index 000000000..f71634475 --- /dev/null +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/error_example.json @@ -0,0 +1,144 @@ +[ + { + "reason": "compiler-message", + "package_id": "passing 0.1.0 (path+file:///home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/compileFail)", + "target": { + "kind": ["lib"], + "crate_types": ["lib"], + "name": "passing", + "src_path": "/home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/compileFail/src/lib.rs", + "edition": "2015" + }, + "message": { + "message": "mismatched types", + "code": { + "code": "E0308", + "explanation": "\nThis error occurs when the compiler was unable to infer the concrete type of a\nvariable. It can occur for several cases, the most common of which is a\nmismatch in the expected type that the compiler inferred for a variable's\ninitializing expression, and the actual type explicitly assigned to the\nvariable.\n\nFor example:\n\n```compile_fail,E0308\nlet x: i32 = \"I am not a number!\";\n// ~~~ ~~~~~~~~~~~~~~~~~~~~\n// | |\n// | initializing expression;\n// | compiler infers type `&str`\n// |\n// type `i32` assigned to variable `x`\n```\n" + }, + "level": "error", + "spans": [ + { + "file_name": "src/lib.rs", + "byte_start": 39, + "byte_end": 42, + "line_start": 2, + "line_end": 2, + "column_start": 39, + "column_end": 42, + "is_primary": true, + "text": [ + { + "text": "pub fn mul_xor_add(a: u64, b: u64) -> u64 {", + "highlight_start": 39, + "highlight_end": 42 + } + ], + "label": "expected u64, found ()", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + }, + { + "file_name": "src/lib.rs", + "byte_start": 8, + "byte_end": 19, + "line_start": 2, + "line_end": 2, + "column_start": 8, + "column_end": 19, + "is_primary": false, + "text": [ + { + "text": "pub fn mul_xor_add(a: u64, b: u64) -> u64 {", + "highlight_start": 8, + "highlight_end": 19 + } + ], + "label": "this function's body doesn't return", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "expected type `u64`\n found type `()`", + "code": null, + "level": "note", + "spans": [], + "children": [], + "rendered": null + }, + { + "message": "consider removing this semicolon", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "src/lib.rs", + "byte_start": 85, + "byte_end": 86, + "line_start": 4, + "line_end": 4, + "column_start": 22, + "column_end": 23, + "is_primary": true, + "text": [ + { + "text": " (x ^ a) + (x ^ b);", + "highlight_start": 22, + "highlight_end": 23 + } + ], + "label": null, + "suggested_replacement": "", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "error[E0308]: mismatched types\n --> src/lib.rs:2:39\n |\n2 | pub fn mul_xor_add(a: u64, b: u64) -> u64 {\n | ----------- ^^^ expected u64, found ()\n | |\n | this function's body doesn't return\n3 | let x = a * b;\n4 | (x ^ a) + (x ^ b);\n | - help: consider removing this semicolon\n |\n = note: expected type `u64`\n found type `()`\n\n" + } + }, + { + "reason": "compiler-message", + "package_id": "passing 0.1.0 (path+file:///home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/compileFail)", + "target": { + "kind": ["lib"], + "crate_types": ["lib"], + "name": "passing", + "src_path": "/home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/compileFail/src/lib.rs", + "edition": "2015" + }, + "message": { + "message": "aborting due to previous error", + "code": null, + "level": "error", + "spans": [], + "children": [], + "rendered": "error: aborting due to previous error\n\n" + } + }, + { + "reason": "compiler-message", + "package_id": "passing 0.1.0 (path+file:///home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/compileFail)", + "target": { + "kind": ["lib"], + "crate_types": ["lib"], + "name": "passing", + "src_path": "/home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/compileFail/src/lib.rs", + "edition": "2015" + }, + "message": { + "message": "For more information about this error, try `rustc --explain E0308`.", + "code": null, + "level": "", + "spans": [], + "children": [], + "rendered": "For more information about this error, try `rustc --explain E0308`.\n" + } + } +] diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/warnings_example.json b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/warnings_example.json new file mode 100644 index 000000000..837407065 --- /dev/null +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/warnings_example.json @@ -0,0 +1,186 @@ +[ + { + "reason": "compiler-message", + "package_id": "passing 0.1.0 (path+file:///home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/warnings)", + "target": { + "kind": ["lib"], + "crate_types": ["lib"], + "name": "passing", + "src_path": "/home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/warnings/src/lib.rs", + "edition": "2015" + }, + "message": { + "message": "unused variable: `z`", + "code": { "code": "unused_variables", "explanation": null }, + "level": "warning", + "spans": [ + { + "file_name": "src/lib.rs", + "byte_start": 138, + "byte_end": 139, + "line_start": 8, + "line_end": 8, + "column_start": 9, + "column_end": 10, + "is_primary": true, + "text": [ + { + "text": " let z = 0;", + "highlight_start": 9, + "highlight_end": 10 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "#[warn(unused_variables)] on by default", + "code": null, + "level": "note", + "spans": [], + "children": [], + "rendered": null + }, + { + "message": "consider prefixing with an underscore", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "src/lib.rs", + "byte_start": 138, + "byte_end": 139, + "line_start": 8, + "line_end": 8, + "column_start": 9, + "column_end": 10, + "is_primary": true, + "text": [ + { + "text": " let z = 0;", + "highlight_start": 9, + "highlight_end": 10 + } + ], + "label": null, + "suggested_replacement": "_z", + "suggestion_applicability": "MachineApplicable", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: unused variable: `z`\n --> src/lib.rs:8:9\n |\n8 | let z = 0;\n | ^ help: consider prefixing with an underscore: `_z`\n |\n = note: #[warn(unused_variables)] on by default\n\n" + } + }, + { + "reason": "compiler-message", + "package_id": "passing 0.1.0 (path+file:///home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/warnings)", + "target": { + "kind": ["lib"], + "crate_types": ["lib"], + "name": "passing", + "src_path": "/home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/warnings/src/lib.rs", + "edition": "2015" + }, + "message": { + "message": "function `xorAdd` should have a snake case name", + "code": { "code": "non_snake_case", "explanation": null }, + "level": "warning", + "spans": [ + { + "file_name": "src/lib.rs", + "byte_start": 90, + "byte_end": 96, + "line_start": 7, + "line_end": 7, + "column_start": 4, + "column_end": 10, + "is_primary": true, + "text": [ + { + "text": "fn xorAdd(x: u64, a: u64, b: u64) -> u64 {", + "highlight_start": 4, + "highlight_end": 10 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": [ + { + "message": "#[warn(non_snake_case)] on by default", + "code": null, + "level": "note", + "spans": [], + "children": [], + "rendered": null + }, + { + "message": "convert the identifier to snake case", + "code": null, + "level": "help", + "spans": [ + { + "file_name": "src/lib.rs", + "byte_start": 90, + "byte_end": 96, + "line_start": 7, + "line_end": 7, + "column_start": 4, + "column_end": 10, + "is_primary": true, + "text": [ + { + "text": "fn xorAdd(x: u64, a: u64, b: u64) -> u64 {", + "highlight_start": 4, + "highlight_end": 10 + } + ], + "label": null, + "suggested_replacement": "xor_add", + "suggestion_applicability": "MaybeIncorrect", + "expansion": null + } + ], + "children": [], + "rendered": null + } + ], + "rendered": "warning: function `xorAdd` should have a snake case name\n --> src/lib.rs:7:4\n |\n7 | fn xorAdd(x: u64, a: u64, b: u64) -> u64 {\n | ^^^^^^ help: convert the identifier to snake case: `xor_add`\n |\n = note: #[warn(non_snake_case)] on by default\n\n" + } + }, + { + "reason": "compiler-artifact", + "package_id": "passing 0.1.0 (path+file:///home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/warnings)", + "target": { + "kind": ["lib"], + "crate_types": ["lib"], + "name": "passing", + "src_path": "/home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/warnings/src/lib.rs", + "edition": "2015" + }, + "profile": { + "opt_level": "0", + "debuginfo": 2, + "debug_assertions": true, + "overflow_checks": true, + "test": false + }, + "features": [], + "filenames": [ + "/home/delma/git/tmc-langs/tmc-langs-rust/src/test/resources/warnings/target/debug/libpassing.rlib" + ], + "executable": null, + "fresh": false + } +] From 3e557263cec3f76130a1741efb4d6d658ae6a3f8 Mon Sep 17 00:00:00 2001 From: Delma Date: Sat, 5 Oct 2019 21:55:23 +0300 Subject: [PATCH 3/4] Made lint parsing work --- .../cs/tmc/langs/rust/CargoPlugin.java | 8 +- .../helsinki/cs/tmc/langs/rust/LintError.java | 29 +---- .../cs/tmc/langs/rust/LinterResult.java | 10 +- .../cs/tmc/langs/rust/LinterResultParser.java | 100 +++++++----------- .../cs/tmc/langs/rust/CargoPluginTest.java | 28 +++-- .../tmc/langs/rust/CargoResultParserTest.java | 6 +- .../rust/CargoStudentFilePolicyTest.java | 4 +- 7 files changed, 75 insertions(+), 110 deletions(-) diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java index b8e1d8bdb..0858ed921 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java @@ -64,7 +64,9 @@ protected StudentFilePolicy getStudentFilePolicy(Path projectPath) { @Override public ValidationResult checkCodeStyle(Path path, Locale locale) { if (run(new String[] {"cargo", "clean"}, path).isPresent()) { - String[] command = {"cargo", "--message-format", "json" , "rustc", "--", "--forbid", "warnings"}; + String[] command = { + "cargo" , "rustc", "--message-format", "json", "--", "--forbid", "warnings" + }; log.info("Building for lints with command {}", Arrays.deepToString(command)); Optional result = run(command, path); if (result.isPresent()) { @@ -152,9 +154,9 @@ private Optional run(String[] command, Path dir) { private RunResult filledFailure(ProcessResult processResult) { byte[] errorOutput = processResult.errorOutput.getBytes(StandardCharsets.UTF_8); ImmutableMap logs = - new ImmutableMap.Builder() + new ImmutableMap.Builder() .put(SpecialLogs.COMPILER_OUTPUT, errorOutput) - .build(); + .build(); return new RunResult(Status.COMPILE_FAILED, ImmutableList.of(), logs); } diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LintError.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LintError.java index b8a8efd7a..78cd2a5f0 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LintError.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LintError.java @@ -2,44 +2,31 @@ import fi.helsinki.cs.tmc.langs.abstraction.ValidationError; -import java.util.List; - public class LintError implements ValidationError { - private final List code; private final int startLine; private final int startColumn; private final String message; private final String file; - private final int endLine; - private final int endColumn; /** * Creates new error that failing lints generate. * * @param file file that the error is in * @param message describes what is wrong - * @param code list of code lines the error is in * @param startLine from what line error start * @param startColumn from what column in that line error starts - * @param endLine in what line error ends - * @param endColumn in what column in that line error ends */ public LintError( String file, String message, - List code, int startLine, - int startColumn, - int endLine, - int endColumn) { - this.code = code; + int startColumn + ) { this.file = file; this.message = message; this.startLine = startLine; this.startColumn = startColumn; - this.endLine = endLine; - this.endColumn = endColumn; } @Override @@ -52,14 +39,6 @@ public int getLine() { return startLine; } - public int getEndColumn() { - return endColumn; - } - - public int getEndLine() { - return endLine; - } - @Override public String getMessage() { return message; @@ -70,8 +49,4 @@ public String getSourceName() { return file; } - public List getCode() { - return code; - } - } diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResult.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResult.java index ee343327d..c50a25cb4 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResult.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResult.java @@ -5,7 +5,7 @@ import fi.helsinki.cs.tmc.langs.abstraction.ValidationResult; import java.io.File; -import java.util.HashMap; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -13,8 +13,8 @@ public class LinterResult implements ValidationResult { private Strategy strategy; private final Map> errors; - public LinterResult() { - errors = new HashMap<>(); + public LinterResult(Map> errors) { + this.errors = errors; strategy = Strategy.DISABLED; } @@ -25,6 +25,6 @@ public Strategy getStrategy() { @Override public Map> getValidationErrors() { - return errors; + return Collections.unmodifiableMap(errors); } -} +} \ No newline at end of file diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java index 1dc9648e8..cec75c633 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/LinterResultParser.java @@ -4,88 +4,66 @@ import fi.helsinki.cs.tmc.langs.abstraction.ValidationResult; import fi.helsinki.cs.tmc.langs.utils.ProcessResult; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import java.io.File; +import java.io.IOException; import java.nio.file.Paths; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Iterator; -import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import java.util.stream.Collector; import java.util.stream.Collectors; import java.util.stream.Stream; public class LinterResultParser { - + private static final ObjectMapper mapper = new ObjectMapper(); - + /** * Parses given process result to a validation result. */ public ValidationResult parse(ProcessResult processResult) { - String output = processResult.errorOutput; + String output = processResult.output; String[] lines = output.split("\\r?\\n"); - List errors = Arrays.stream(lines) - .map((line) -> mapper.readTree(line)) - .filter((parseTree) -> parseTree.get("reason").asText().equals("compiler-message")) - .flatMap((parseTree) -> { - for (JsonNode node : parseTree.get("message").get("spans")) { + return new LinterResult(Arrays + .stream(lines) + .filter(line -> line.startsWith("{")) + .flatMap(line -> { + try { + return Stream.of(mapper.readTree(line)); + } catch (IOException e) { + // TODO: When does IOException happen? Or does it? + throw new RuntimeException(e); + } + }) + .filter(parseTree -> parseTree + .get("reason") + .asText() + .equals("compiler-message") + ) + .map(parseTree -> parseTree.get("message")) + .flatMap(parseTree -> { + for (JsonNode node : parseTree.get("spans")) { if (node.get("is_primary").asBoolean()) { - return Stream.of(createLintError(node, parseTree.get("rendered").asText())); + return Stream.of(createLintError( + node, + parseTree.get("rendered").asText() + )); } } return Stream.empty(); }) - .collect(Collectors.toList()); - - - // LinterResult result = new LinterResult(); - // List code = new ArrayList<>(); - // for (String line : lines) { - // Matcher matcher = ERROR_START.matcher(line); - // if (matcher.matches()) { - // code = new ArrayList<>(); - // addLintError(matcher, code, result); - // } else { - // matcher = ERROR_CONTINUE.matcher(line); - // if (matcher.matches()) { - // code.add(line); - // } else { - // matcher = ARROW.matcher(line); - // if (!matcher.matches()) { - // break; - // } - // } - // } - // } - return result; - } - - private LintError createLintError(JsonNode node, String message) { - String file = node.get(fieldName) - return new LintError(file, message, code, startLine, startColumn, endLine, endColumn); + .collect(Collectors.groupingBy(error -> + Paths.get(error.getSourceName()).toFile() + )) + ); } - private void addLintError(Matcher matcher, List code, LinterResult result) { - String fileName = matcher.group("file"); - LintError current = new LintError( - fileName, - matcher.group("description"), - code, - Integer.parseInt(matcher.group("startLine")), - Integer.parseInt(matcher.group("startColumn")), - Integer.parseInt(matcher.group("endLine")), - Integer.parseInt(matcher.group("endColumn"))); - File file = Paths.get(fileName).toFile(); - List errors = result.getValidationErrors().get(file); - if (errors == null) { - errors = new ArrayList<>(); - result.getValidationErrors().put(file, errors); - } - errors.add(current); + private ValidationError createLintError(JsonNode node, String message) { + return new LintError( + node.get("file_name").asText(), + message, + node.get("line_start").asInt(), + node.get("column_start").asInt() + ); } } diff --git a/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoPluginTest.java b/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoPluginTest.java index 9e36c2129..a6272c750 100644 --- a/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoPluginTest.java +++ b/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoPluginTest.java @@ -22,11 +22,13 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Map.Entry; +import java.util.stream.Collectors; public class CargoPluginTest { @@ -56,7 +58,7 @@ public void testProjectWithPassingTestCompilesAndPassesTests() { assertEquals(RunResult.Status.PASSED, result.status); assertEquals(1, result.testResults.size()); - assertTrue(result.testResults.get(0).passed); + assertTrue(result.testResults.get(0).isSuccessful()); } @Test @@ -66,8 +68,8 @@ public void testProjectWithMultiplePassingTestCompilesAndPassesTests() { assertEquals(RunResult.Status.PASSED, result.status); assertEquals(2, result.testResults.size()); - assertTrue(result.testResults.get(0).passed); - assertTrue(result.testResults.get(1).passed); + assertTrue(result.testResults.get(0).isSuccessful()); + assertTrue(result.testResults.get(1).isSuccessful()); } @Test @@ -77,7 +79,7 @@ public void testProjectWithFailingTestCompilesAndFailsTest() { assertEquals(RunResult.Status.TESTS_FAILED, result.status); assertEquals(1, result.testResults.size()); - assertFalse(result.testResults.get(0).passed); + assertFalse(result.testResults.get(0).isSuccessful()); } @Test @@ -87,11 +89,11 @@ public void testProjectPartiallyFailingTestCompilesAndFailsTest() { assertEquals(RunResult.Status.TESTS_FAILED, result.status); assertEquals(2, result.testResults.size()); - boolean first = result.testResults.get(0).passed; + boolean first = result.testResults.get(0).isSuccessful(); if (first) { - assertFalse(result.testResults.get(1).passed); + assertFalse(result.testResults.get(1).isSuccessful()); } else { - assertTrue(result.testResults.get(1).passed); + assertTrue(result.testResults.get(1).isSuccessful()); } } @@ -102,7 +104,7 @@ public void testTryingToCheatByAddingTestFails() { assertEquals(RunResult.Status.TESTS_FAILED, result.status); assertEquals(1, result.testResults.size()); - assertFalse(result.testResults.get(0).passed); + assertFalse(result.testResults.get(0).isSuccessful()); } @Test @@ -135,7 +137,7 @@ public void lintingHasRightErrorWithOneError() { .getValidationErrors().values() .iterator().next().get(0); assertEquals(7, validation.getLine()); - assertEquals(1, validation.getColumn()); + assertEquals(4, validation.getColumn()); assertTrue(validation.getMessage().contains("snake case")); assertTrue(validation.getSourceName().contains("lib.rs")); } @@ -179,6 +181,14 @@ public void lintingWorksWithTwoFiles() { Path path = TestUtils.getPath(getClass(), "warningFiles"); ValidationResult result = cargoPlugin.checkCodeStyle(path, new Locale("en")); Map> errors = result.getValidationErrors(); + // TODO: Debug + errors.entrySet().stream() + .forEach((e) -> + System.out.println("key: " + e.getKey() + ", value: " + e.getValue() + .stream() + .map(t -> t.getMessage()) + .collect(Collectors.joining())) + ); assertEquals(2, errors.size()); Iterator>> errorIt = errors.entrySet().iterator(); Entry> error1 = errorIt.next(); diff --git a/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoResultParserTest.java b/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoResultParserTest.java index a01d96714..c32f49162 100644 --- a/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoResultParserTest.java +++ b/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoResultParserTest.java @@ -50,9 +50,9 @@ public void failFails() { public void failContainsRightResult() { RunResult parsed = parser.parse(failResult); assertEquals(1, parsed.testResults.size()); - assertEquals("description", parsed.testResults.get(0).errorMessage); - assertEquals("name", parsed.testResults.get(0).name); - assertEquals(false, parsed.testResults.get(0).passed); + assertEquals("description", parsed.testResults.get(0).getMessage()); + assertEquals("name", parsed.testResults.get(0).getName()); + assertEquals(false, parsed.testResults.get(0).isSuccessful()); } @Test diff --git a/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoStudentFilePolicyTest.java b/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoStudentFilePolicyTest.java index db60ab124..c2ec366fe 100644 --- a/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoStudentFilePolicyTest.java +++ b/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoStudentFilePolicyTest.java @@ -19,11 +19,11 @@ public void setUp() { @Test public void testCargoTomlIsNotStudentFile() { - assertFalse(policy.isStudentSourceFile(Paths.get("src", "Cargo.toml"))); + assertFalse(policy.isStudentSourceFile(Paths.get("src", "Cargo.toml"), Paths.get(""))); } @Test public void testSourceFileIsSourceFile() { - assertTrue(policy.isStudentSourceFile(Paths.get("src", "hello.rs"))); + assertTrue(policy.isStudentSourceFile(Paths.get("src", "hello.rs"), Paths.get(""))); } } From e1695640f289185ae60c9ff4d93a4fd0b63cfa0a Mon Sep 17 00:00:00 2001 From: Delma Date: Sat, 12 Oct 2019 21:04:32 +0300 Subject: [PATCH 4/4] When doing linting first build normally to get all warnings and then forbidding warnings to prevent cheating --- .../cs/tmc/langs/rust/CargoPlugin.java | 33 +++++++++++++------ .../cs/tmc/langs/rust/CargoPluginTest.java | 8 ----- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java index 0858ed921..c0eb88e6b 100644 --- a/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java +++ b/tmc-langs-rust/src/main/java/fi/helsinki/cs/tmc/langs/rust/CargoPlugin.java @@ -36,19 +36,21 @@ public class CargoPlugin extends AbstractLanguagePlugin { private static final Logger log = LoggerFactory.getLogger(CargoPlugin.class); private static final RunResult EMPTY_FAILURE = new RunResult( - Status.COMPILE_FAILED, - ImmutableList.of(), - new ImmutableMap.Builder().build()); + Status.COMPILE_FAILED, + ImmutableList.of(), + new ImmutableMap.Builder().build() + ); /** * Creates new plugin for cargo with all default stuff set. */ public CargoPlugin() { super( - new ExerciseBuilder(), - new StudentFileAwareSubmissionProcessor(), - new StudentFileAwareZipper(), - new StudentFileAwareUnzipper()); + new ExerciseBuilder(), + new StudentFileAwareSubmissionProcessor(), + new StudentFileAwareZipper(), + new StudentFileAwareUnzipper() + ); } @Override @@ -65,12 +67,23 @@ protected StudentFilePolicy getStudentFilePolicy(Path projectPath) { public ValidationResult checkCodeStyle(Path path, Locale locale) { if (run(new String[] {"cargo", "clean"}, path).isPresent()) { String[] command = { - "cargo" , "rustc", "--message-format", "json", "--", "--forbid", "warnings" + "cargo" , "rustc", "--message-format", "json" }; log.info("Building for lints with command {}", Arrays.deepToString(command)); Optional result = run(command, path); if (result.isPresent()) { - return parseLints(result.get()); + ValidationResult validationResult = parseLints(result.get()); + if (validationResult.getValidationErrors().isEmpty()) { + String[] command2 = { + "cargo" , "rustc", "--message-format", "json", "--", "--forbid", "warnings" + }; + log.info("Ensuring that lints are not being dissabled {}", Arrays.deepToString(command2)); + Optional result2 = run(command, path); + if (result2.isPresent()) { + return parseLints(result2.get()); + } + } + return validationResult; } } log.error("Build for lints failed."); @@ -78,7 +91,7 @@ public ValidationResult checkCodeStyle(Path path, Locale locale) { } public String getLanguageName() { - return "cargo"; + return "Rust"; } public String getPluginName() { diff --git a/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoPluginTest.java b/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoPluginTest.java index a6272c750..64d3c509c 100644 --- a/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoPluginTest.java +++ b/tmc-langs-rust/src/test/java/fi/helsinki/cs/tmc/langs/rust/CargoPluginTest.java @@ -181,14 +181,6 @@ public void lintingWorksWithTwoFiles() { Path path = TestUtils.getPath(getClass(), "warningFiles"); ValidationResult result = cargoPlugin.checkCodeStyle(path, new Locale("en")); Map> errors = result.getValidationErrors(); - // TODO: Debug - errors.entrySet().stream() - .forEach((e) -> - System.out.println("key: " + e.getKey() + ", value: " + e.getValue() - .stream() - .map(t -> t.getMessage()) - .collect(Collectors.joining())) - ); assertEquals(2, errors.size()); Iterator>> errorIt = errors.entrySet().iterator(); Entry> error1 = errorIt.next();