Skip to content

Improved syntastic linting #76

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

Closed
wants to merge 3 commits into from
Closed

Improved syntastic linting #76

wants to merge 3 commits into from

Conversation

somini
Copy link

@somini somini commented Mar 21, 2016

This PR builds on PR #72 .

There's a new option to choose between fast lint (parse-only) or comprehensive (no-trans). You can change this at runtime too.

Possible improvements include a buffer-local variable so that you can have different projects with different linting levels.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @chris-morgan (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

let makeprg = self.makeprgBuild({ 'args': '-Zparse-only' })
let compiler_params = g:rustc_syntax_only ? '-Zparse-only' : '-Zno-trans'
let cwd = '.' " Don't change cwd as default
let cargo_toml_path = findfile('Cargo.toml', '.;')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this search in parent directories or only in current dir and subdirs? Thanks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the current and parent directories until it finds the file.
See :help findfile()

@Limeth
Copy link

Limeth commented May 6, 2016

I have been using this for a while and it seems to be working fine. 👍

@somini
Copy link
Author

somini commented May 7, 2016

Can I rebase this to the latest master, or should I wait for more feedback?
I'm not sure what the rules regarding this are.

@steveklabnik
Copy link
Member

steveklabnik commented May 7, 2016

@somini ah, sorry about this! It can be tough to know with the vim repo sometimes.

I think this looks good to me; if you rebase it, i think we can land it.

@somini
Copy link
Author

somini commented May 8, 2016

There's actually no conflicts, so it's OK to merge this.

@ernestrc
Copy link

ernestrc commented Jun 19, 2016

@somini FYI. I turned debugging on for syntastic, because you're branch was not working for me. I get a status code 101 from rustc but no errors from syntastic:

syntastic: 6.066680: UpdateErrors (auto): default checkers
syntastic: 6.067186: CacheErrors: default checkers
syntastic: 6.068247: g:syntastic_aggregate_errors = 0
syntastic: 6.068530: getcwd() = '/home/ernestrc/dev/sonicd'
syntastic: 6.069081: CacheErrors: Invoking checker: rust/rustc
syntastic: 6.070460: SyntasticMake: called with options: {'cwd': '/home/ernestrc/dev/soni
cd', 'errorformat': '%E%f:%l:%c: %\d%#:%\d%# %.%\{-}error:%.%\{-} %m,%W%f:%l:%c: %\d%#:%\
d%# %.%\{-}warning:%.%\{-} %m,%C%f:%l %m,%-Z%.%#', 'makeprg': 'cargo rustc -Zno-trans'}
syntastic: 6.425845: system: command run in 0.354779s
syntastic: 6.426819: getLocList: checker rust/rustc returned 101
syntastic: 6.427252: getLocList: checker rust/rustc run in 0.357960s

I checked the output of cargo rustc -Zno-trans:

$ cargo rustc -Zno-trans
error: extra arguments to `rustc` can only be passed to one target, consider filtering  
the package by passing e.g. `--lib` or `--bin NAME` to specify a single target

EDIT:

$ rustup show
nightly-x86_64-unknown-linux-gnu (directory override for '/home/ernestrc/dev/sonicd')
rustc 1.11.0-nightly (bb4a79b08 2016-06-15)

@somini
Copy link
Author

somini commented Jun 20, 2016

I installed the nightly version and can confirm @ernestrc report. It works on 1.9.

I can "fix" it by adding the --lib argument. So cargo rustc --lib -Zno-trans works, but only because I don't know the bin name.

I really don't know enough Rust to really understand what's going on here, so any help is appreciated.

@somini
Copy link
Author

somini commented Jun 25, 2016

Rebased this to the latest master, it was starting to diverge too much.

Still can't fix the breakage that occured on rust v1.11.

@steveklabnik
Copy link
Member

Still interested in landing this, someone was just asking on IRC.

What was that breakage, exactly? Sorry for being out of the loop.

@somini
Copy link
Author

somini commented Aug 26, 2016

Thanks for keeping this alive.

The problem is that cargo can't pass extra arguments to rustc unless we specify a binary or the lib option. This was introduced between 1.9 and 1.11.
What we need is to replace this command cargo rustc -Zno-trans with the correct invocation for cargo to compile the project with the given rust flags.

@euclio
Copy link

euclio commented Sep 7, 2016

According to rust-lang/cargo#2702 it seems like the right thing to do here is either default to --lib or parse cargo metadata and run cargo rustc once for each target.

@somini
Copy link
Author

somini commented Sep 7, 2016

@euclio led me to this.
https://github.com/Manishearth/rust-clippy/issues/935#issuecomment-219547089
Might be easier to implement in VimL than parsing stuff.

@euclio
Copy link

euclio commented Sep 7, 2016

Recent versions of vim (7.4.1154+ renamed in 7.4.1304) have jsondecode({string}) and json_decode({string}).

@Limeth
Copy link

Limeth commented Oct 4, 2016

Any updates on this? It would be a really useful feature. I've been using @somini's fork for quite some time and now I almost can't code without it. :)

@nateozem
Copy link

I'm not quite sure if anyone had updated their rustc to version 1.13.0, but using the -Z flags will now give a warning. Hopefully there would be a variable to set for the plugin to indicate if using nightly version.
Warning:

warning: the option `Z` is unstable and should only be used on the nightly compiler, but it is currently accepted for backwards compatibility; this will soon change, see issue #31847 for more detail
s

@mckinnsb
Copy link

I decided to put mention my fork ( https://github.com/mckinnsb/rust.vim ) here, because I figured that it might be of use to anyone who needs more than just parsing checks using rustc.

I used @jFransham's fork(https://github.com/jFransham/rust.vim) as a template, and then modified his cargo checker to use the same error format as the updated rustc checker in rust.vim. It basically runs "cargo rustc" in the directory of the file you are editing.

I'm not sure if I should make a PR with this or not, but I did make the changes on master and syntax checking with external crates is now working, although it is probably slower than simple parsing.

You enable it by setting the syntastic checker:

let g:syntastic_rust_checkers = ['cargo'] 

@jFransham
Copy link

Oh hey, this is a nice surprise

@ghost
Copy link

ghost commented Apr 28, 2017

What I'm currently using: patch1 and patch2 - tested

But I should probably be using #147 which seems to be the same as the above #76 (comment) - not tested

EDIT: my .vimrc

@jacwah
Copy link

jacwah commented Aug 1, 2017

As far as I understand it, using cargo-check should solve this.

@ghost ghost mentioned this pull request Jan 5, 2018
\ 'args': 'rustc ' . compiler_params,
\ 'fname': '' })
" Change cwd to the root of the crate
let cwd = fnamemodify( cargo_toml_path, ':p:h')
Copy link

@ghost ghost Jan 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line currently doesn't work inside a workspace, for it needs to find workspace's Cargo.toml (which is ../ relative to what it currently finds), so reported filenames in the errors are like:
--> recompile_self/build.rs:32:3
but cwd is
/home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage/recompile_self
when it should be
/home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage
for vim/syntastic to actually show the errors upon save.

I don't know of a way to detect if we're inside a workspace now...

EDIT: I opted to patch cargo to pass absolute path names to rustc so that they will show up as such in the error messages, and thus not need any cwd changes!

@da-x
Copy link
Member

da-x commented Jul 5, 2018

There were various fixes to the Cargo checker in rust.vim (you can try it out, or switch to async checkers such as ALE, or neomake, plus the way cargo is invoked for checking projects has changed and improved in the last two years.

If relevant, please re-open. Thanks!

@da-x da-x closed this Jul 5, 2018
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 this pull request may close these issues.