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

Clippy warning about an almost complete ascii range #1070

Open
xamgore opened this issue Jan 28, 2025 · 4 comments
Open

Clippy warning about an almost complete ascii range #1070

xamgore opened this issue Jan 28, 2025 · 4 comments
Labels

Comments

@xamgore
Copy link

xamgore commented Jan 28, 2025

I am running cargo clippy over async-graphql crate which uses pest v2.7.11.

warning: almost complete ascii range
    --> parser/src/parse/generated.rs:2312:29
     |
2312 |           state.match_range('0'..'9')
     |                             ^^^--^^^
     |                                |
     |                                help: use an inclusive range: `..=`
     |
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range
     = note: `#[warn(clippy::almost_complete_range)]` on by default

As we can see at vm.rs and generator.rs, range is included in fact.

// vm
"ASCII_DIGIT" => return state.match_range('0'..'9'),

// generated
insert_builtin!(builtins, ASCII_DIGIT, state.match_range('0'..'9'));

pub(crate) fn match_range(&mut self, range: Range<char>) -> bool {
    if let Some(c) = self.input[self.pos..].chars().next() {
        if range.start <= c && c <= range.end {   // <<<<<<<<<<< here
            self.pos += c.len_utf8();
            return true;
        }
    }

    false
}

As pest uses 0..9 just for expressiveness, I would either mute clippy at the generated code, or switched to 0..=9 (affects MSRV).

@xamgore xamgore added the bug label Jan 28, 2025
@tomtau
Copy link
Contributor

tomtau commented Jan 29, 2025

Yes, that's an old choice in Pest that it matches ranges inclusively and changing it would be a breaking change. @xamgore you can mute the clippy warning yourself, or do you suggest adding the clippy ignore in the generated code?

@xamgore
Copy link
Author

xamgore commented Jan 29, 2025

I think it should be in the generated code, yeah. I have encountered this at async-graphql, it's not the first it's not the last crate.

@tomtau
Copy link
Contributor

tomtau commented Jan 29, 2025

@xamgore the generated code should have this: https://github.com/pest-parser/pest/blob/master/generator/src/generator.rs#L62 -- I guess it's not applied to it @xamgore ?

@xamgore
Copy link
Author

xamgore commented Jan 29, 2025

Thanks for the reply. I'll check, is it a problem of an outdated pest version being used, a problem in async-graphql itself, or anything else.

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

No branches or pull requests

2 participants