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

Error linting project using wasm-bindgen on nighlty #4507

Closed
alexlapa opened this issue Sep 5, 2019 · 6 comments
Closed

Error linting project using wasm-bindgen on nighlty #4507

alexlapa opened this issue Sep 5, 2019 · 6 comments
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied T-macros Type: Issues with macros and macro expansion

Comments

@alexlapa
Copy link

alexlapa commented Sep 5, 2019

cargo clippy --all -- -D clippy::pedantic -D warnings is ok

running cargo +nightly clippy --all -- -D clippy::pedantic -D warnings produces:

error: integer type suffix should be separated by an underscore
  --> jason/src/api/connection.rs:28:1
   |
28 | #[wasm_bindgen]
   | ^^^^^^^^^^^^^^^ help: add an underscore: `#[wasm_bindg_u32`
   |
   = note: `-D clippy::unseparated-literal-suffix` implied by `-D clippy::pedantic`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unseparated_literal_suffix
cargo clippy -V 
clippy 0.0.212 (e3cb40e 2019-06-25)
cargo +nightly clippy -V
clippy 0.0.212 (aeadf15 2019-09-03)

Reproduction.

@flip1995
Copy link
Member

flip1995 commented Sep 6, 2019

cc #4421 @lzutao

It seems that

if let Some(firstch) = src.chars().next();
if char::to_digit(firstch, 10).is_some();

was necessary. I think, we should add this back in, instead of the

let lit_snip = match snippet_opt(cx, lit.span) {
Some(snip) => {
// The snip could be empty in case of expand from procedure macro
if snip.is_empty() || snip.contains('!') {
return;
}

hack. But this time we should document, why this exists.

@flip1995 flip1995 added L-suggestion Lint: Improving, adding or fixing lint suggestions I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied good-first-issue These issues are a good way to get started with Clippy C-bug Category: Clippy is not doing the correct thing T-macros Type: Issues with macros and macro expansion and removed L-suggestion Lint: Improving, adding or fixing lint suggestions labels Sep 6, 2019
@tesuji
Copy link
Contributor

tesuji commented Sep 6, 2019

I added it back in #4510. But unfortunately, proc-macro is out of my league.
It would be nice if someone could add a regression test for this.

@flip1995
Copy link
Member

flip1995 commented Sep 6, 2019

@alexlapa Can you provide a minimal reproducible example or share the code this lint triggered on?

@alexlapa
Copy link
Author

alexlapa commented Sep 6, 2019

@flip1995 ,

Sure. https://github.com/alexlapa/rust-clippy-4507-reproduction

@flip1995
Copy link
Member

flip1995 commented Sep 6, 2019

Thanks! Extract from expanded code:

        // ...
        unsafe fn __wbg_foo_new(_: u32) -> u32 {
            {
                ::std::rt::begin_panic("cannot convert to JsValue outside of the wasm target",
                                       &("src/lib.rs", 3u32, 1u32))
            }
        }
        // ...

It seems, we only have to write a proc macro which produces code with 0u32 (or similar) in it. This should be easily done by extending

#[proc_macro_derive(ClippyMiniMacroTest)]
pub fn mini_macro(_: TokenStream) -> TokenStream {

@lzutao do you want to take a shot at this? Otherwise I can add a test case to the PR this weekend.

bors added a commit that referenced this issue Sep 9, 2019
…flip1995

Fix regression in case of proc-macro attribute expansion

cc  #4507
changelog: none
r? @flip1995
@tesuji
Copy link
Contributor

tesuji commented Sep 10, 2019

Fixed in #4510.

@phansch phansch closed this as completed Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied T-macros Type: Issues with macros and macro expansion
Projects
None yet
Development

No branches or pull requests

4 participants