Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The new error format is not supported #26

Closed
blabaere opened this issue Sep 16, 2016 · 10 comments · Fixed by #30
Closed

The new error format is not supported #26

blabaere opened this issue Sep 16, 2016 · 10 comments · Fixed by #30

Comments

@blabaere
Copy link

With rust nightly the new error format is now active by default and the current regex cannot match both the old and new formats.

@oschwald
Copy link
Owner

Thanks for the report. We should likely switch to the new JSON output format as that should be much easier to parse:

cargo rustc --  -Zno-trans -Zunstable-options --error-format=json

@FichteFoll
Copy link

FichteFoll commented Nov 10, 2016

I managed to hack basic support for the new format together like follows:

import json
# [...]

class Rust(Linter):
    # [...]
    cmd = ['rustc', '--error-format=json']
    regex = (r'^(?P<line>.*)$')

    # [...]

    def split_match(self, match):
        # [...]
        data = json.loads(match.group('line'))
        if not data['spans']:
            return (None,) * 7
        span = data['spans'][0]
        matched_file = span['file_name']

        if matched_file:
            if self.use_cargo:
                path = self.cargo_config
            elif self.use_crate_root:
                path = self.crate_root
            else:
                path = False

            if path:
                working_dir = os.path.dirname(path)
                if not self.is_current_file(working_dir, matched_file):
                    return (None,) * 7

        if span['text']:
            text = span['text'][0]
            near = text['text'][text['highlight_start']:text['highlight_end']]
        else:
            near = None

        return (match,
                span['line_start'] - 1,
                span['column_start'] - 1,
                data['level'].endswith("error"),
                data['level'] == 'warning',
                data['message'],
                None)

I suppose that could be of use to you.

I don't have enough experience with the output format or with cargo to prepare an actual pull request, however. For example, I don't know if all cargo ... things also report like this (especially cargo check) and what should happen if "spans" has no elements or more than one.

2016-11-11_01-12-09_overwolf

@mandeep
Copy link
Contributor

mandeep commented Mar 17, 2017

@FichteFoll thanks for sharing your workaround. It works for me when editing a standalone .rs file, however I can't seem to get it to work on Cargo projects. I've read the RFC on the new error format and it seems that the following regex would work:

r'((?P<error>(error|fatal error))|(?P<warning>warning)):\s+'
r'(?P<message>.+)\s+-->\s+(?P<file>.+?):(?P<line>\d+):(?P<col>\d+)'

Unfortunately, changing the regex in linter.py doesn't seem to have an effect. I would think that something needs to change in split_match(), but I don't know enough about sublimelinter plugins to investigate further. Any suggestions?

@oschwald
Copy link
Owner

If someone wants to submit a PR, I'd be happy to accept. I am curious what advantages this package has over Sublime Rust though.

@mandeep
Copy link
Contributor

mandeep commented Mar 17, 2017

The Rust Enhanced plugin feels like a non-native Sublime Text plugin. Its error messages look out of place, and the plugin has problems where it resets back to the Rust package. In my opinion, sublimelinter-contrib-rustc works best for Rust projects and it'd be a shame to see it lose maintainability.

@mandeep
Copy link
Contributor

mandeep commented Mar 17, 2017

I realized what the problem was with the new regex and have submitted a pull request that fixes this issue. Let me know what you think.

@FichteFoll
Copy link

I would recommend utilizing the json error output format intended to be parsed by machines instead of the human-readable output format, but whatever floats your boat.

@oschwald
Copy link
Owner

@FichteFoll, I agree that would be an improvement. However, the above PR and release was an immediate fix for this issue. Feel free to open a PR to switch the linter to use the JSON output.

@FichteFoll
Copy link

If I was using Rust and this package more frequently, I would, but as I mentioned earlier I don't consider myself to have the expertise necessary to judge whether my changes posted above are sufficient in all occasions, as @mandeep already seemed to be running into.

@mandeep
Copy link
Contributor

mandeep commented Mar 18, 2017

I agree that parsing the JSON output would be less error prone, and I hope to submit a PR that uses the JSON error format soon.

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

Successfully merging a pull request may close this issue.

4 participants