Skip to content
This repository has been archived by the owner on Apr 3, 2018. It is now read-only.

Rust error format updates and IDE support #130

Closed
sophiajt opened this issue Aug 8, 2016 · 16 comments
Closed

Rust error format updates and IDE support #130

sophiajt opened this issue Aug 8, 2016 · 16 comments

Comments

@sophiajt
Copy link

sophiajt commented Aug 8, 2016

We've been working to create a new error output format. These initial tests have gone well, and we're currently discussing making this the default error format. Once this switch is thrown, there are still some weeks before this gets released in a stable release. We hope to make the transition as smooth as possible for IDE plugins.

The PR mentioned above also enables a supported JSON output mode that contains the information used in the error message. We're hoping this mode will give plugins more options to choose from for how the support errors.

Feel free to jump in on the thread if this impacts your plugin, and let us know how we can help.

@tupshin
Copy link

tupshin commented Aug 11, 2016

I just came here to file a bug against RustDT, if there wasn't one already. I am using nightly, to take advantage of a number of features, and just did a "rustup update" which got me the new error format. And yes, RustDT fails to detect errors at all now, so nothing is populated in the Tasks pane of eclipse. I would love a fix to this, as these new error messages have already helped me track down a couple of errors that had been plaguing me for days. Thanks for that, btw. :D

@bruno-medeiros
Copy link
Contributor

@PieterPenninckx
Copy link
Contributor

I'm currently working on this.

@bruno-medeiros
Copy link
Contributor

I'm currently working on this.

Cool, I've been busy with loads of stuff, haven't been able to look into this.

@sophiajt
Copy link
Author

Great! I was trying to remember to mention that we're about 16 days away from the Rust 1.12, which will have the new errors on by default (and the old error format removed). It also has stable JSON, so that's also available.

@PieterPenninckx
Copy link
Contributor

Ok. I created a branch that works for me at least for the nightly version of Rust that I downloaded some time ago. I will have to re-test with a more recent version. It parses the Json format. The parser for the old format is still there. Based upon the compiler flags (environment variable, actually), it determines which parser to use. In Eclipse, the user has to set the environment variable RUSTFLAGS to --error-format json manually, otherwise the old parser is still used.

I've been busy with other things this week and I will still be busy today. But for tomorrow, I plan to re-test with a more recent version of the compiler and to propose a more user-friendly solution to determine which error parser to use.

@PieterPenninckx
Copy link
Contributor

PieterPenninckx commented Sep 15, 2016

It still works with the most recent nightly :-)

I have a proposal for deciding which error output format to use (edit: see also my comment from October 3):

  • In the default "Build Command": append the environment variable RUSTFLAGS to the native environment. This environment variable has as default value the substitution variable {RUST_ERROR_OUTPUT_FORMAT}
  • For each compile:
  1. Launch rustc --version and parse the output to get the version number.
  2. Use this to determine a substitution variable {RUST_ERROR_OUTPUT_FORMAT}: the empty string for versions up to and including 1.11 and --error-format json for version 1.12 and higher.

@bruno-medeiros is this ok for you?

I don't know how much time I will have for this, so I'll focus on creating a pull request for the not-so-user-friendly solution first.

Edit: fixed version numbers as pointed out by jonathandturner.

@sophiajt
Copy link
Author

Just one note - 1.12 will be the first with new errors and stable JSON, not 1.13. (what's in nightly now will become beta for a cycle and then be released sometime in November)

@PieterPenninckx
Copy link
Contributor

It seems like I won't find the time to write code for automatically switching to the correct output parser. Maybe better leave as-is, and just update the default settings for the environment variable so that it works out-of-the-box for rustc 1.12 and above + add some documentation how you can change the environment variable settings if you want to use rustc 1.11 or below.

@bruno-medeiros
Copy link
Contributor

I was just looking at rust-lang/cargo#2982 to get acquanted with how the new error format works.

And yeah, @PieterPenninckx don't bother trying to detect correct output parser. Just make it work for rustc 1.2 (which is out now, and I imagine the vast majority of people will be using 1.2 or above). If someone happens to need to use 1.11, they can modify the environment variable settings, like you said. The RustDT parser can just detect which one format to use (for example, if line begin with "{", it's json, otherwise, old format. According to RustyCode devs, that's what they've been doing, so it works.)

Anyways, onwards to reviewing the PR

@bruno-medeiros
Copy link
Contributor

Thanks @PieterPenninckx for the PR, now just needs some more testing before a release is made.

If anyone wants to try it out already, you can build RustDT yourselves, all you need is Maven (https://github.com/RustDT/RustDT#automated-building-and-testing), and you will get a local update site

@tupshin
Copy link

tupshin commented Oct 11, 2016

I did a fresh checkout of the source a couple of minutes ago, and mvn -e verify gives me
=============================== RustBuildOutputParserJsonTest ===============================
Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.01 sec <<< FAILURE! - in com.github.rustdt.tooling.RustBuildOutputParserJsonTest
testCargoErrorMesages(com.github.rustdt.tooling.RustBuildOutputParserJsonTest) Time elapsed: 0.001 sec
test(com.github.rustdt.tooling.RustBuildOutputParserJsonTest) Time elapsed: 0.008 sec <<< ERROR!
melnorme.utilbox.core.Assert$AssertFailedException: null
at melnorme.utilbox.core.Assert$AssertHandler.handleAssert(Assert.java:62)
at melnorme.utilbox.core.Assert.checkAssertion(Assert.java:94)
at melnorme.utilbox.core.Assert$AssertNamespace.assertFail(Assert.java:190)
at com.github.rustdt.tooling.RustBuildOutputParserJsonTest$TestsRustBuildOutputParserJson.handleParseError(RustBuildOutputParserJsonTest.java:33)
at melnorme.lang.tooling.toolchain.ops.BuildOutputParser2.handleMessageParseError(BuildOutputParser2.java:101)
at com.github.rustdt.tooling.RustBuildOutputParserJson.doParseToolMessage(RustBuildOutputParserJson.java:71)
at melnorme.lang.tooling.toolchain.ops.BuildOutputParser2.parseOutput(BuildOutputParser2.java:76)
at melnorme.lang.tooling.toolchain.ops.BuildOutputParser2.parseOutput(BuildOutputParser2.java:68)
at com.github.rustdt.tooling.RustBuildOutputParserJsonTest.testParseMessages(RustBuildOutputParserJsonTest.java:137)
at com.github.rustdt.tooling.RustBuildOutputParserJsonTest.test$(RustBuildOutputParserJsonTest.java:56)
at com.github.rustdt.tooling.RustBuildOutputParserJsonTest.test(RustBuildOutputParserJsonTest.java:38)

@bruno-medeiros
Copy link
Contributor

Yeah I forgot to run the tests before commiting 😅 , but it's fixed now: https://travis-ci.org/RustDT/RustDT/builds/166805327

@bruno-medeiros
Copy link
Contributor

Note that if Cargo starts building multiple targets, it is likely RustDT with fail to parse the messages due to this issue: rust-lang/rust#37011
I've opened #138 - ideally we'd get that done before next RustDT release.

@tupshin
Copy link

tupshin commented Oct 11, 2016

Thanks much for this. I am still getting a test error when running verify, but I can also confirm that this version does work with the new format and is picking up my errors, displaying them in the problems tab, etc.

===============================  CargoManifest_Test  ===============================
Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.014 sec <<< FAILURE! - in com.github.rustdt.tooling.cargo.CargoManifest_Test
testName(com.github.rustdt.tooling.cargo.CargoManifest_Test)  Time elapsed: 0.001 sec
testEffectiveBinaries(com.github.rustdt.tooling.cargo.CargoManifest_Test)  Time elapsed: 0.008 sec  <<< ERROR!
melnorme.utilbox.core.Assert$AssertFailedException: null
    at melnorme.utilbox.core.Assert$AssertHandler.handleAssert(Assert.java:62)
    at melnorme.utilbox.core.Assert.checkAssertion(Assert.java:94)
    at melnorme.utilbox.core.Assert$AssertNamespace.assertTrue(Assert.java:157)
    at melnorme.utilbox.tests.CommonTestUtils.assertAreEqual(CommonTestUtils.java:57)
    at com.github.rustdt.tooling.cargo.CargoManifest_Test.testEffectiveTargets(CargoManifest_Test.java:92)
    at com.github.rustdt.tooling.cargo.CargoManifest_Test.testEffectiveBinaries$(CargoManifest_Test.java:69)
    at com.github.rustdt.tooling.cargo.CargoManifest_Test.testEffectiveBinaries(CargoManifest_Test.java:51)

@bruno-medeiros
Copy link
Contributor

@tupshin That's odd, my my local tests (Windows machine) and the Travis-CI build complete successfully, no test failures.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants