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

Don't ignore warnings in html5ever/src/tree_builder #551

Merged
merged 16 commits into from
Aug 31, 2024

Conversation

simonwuelker
Copy link
Contributor

Currently, html5ever/src/tree_builder/mod.rs contains this:

#![allow(warnings)]

Presumably, this is done to ignore all the warnings caused by the rules.rs file generated at build time (Which is simply included in mod.rs).

However, this masks many warnings from the actual code itself. This PR instead puts the generated code into its own module, ignores warnings only for this specific module and then fixes (almost) all the warnings in the rest of the code.

#[allow(warnings)]
mod autogenerated {
    include!(concat!(env!("OUT_DIR"), "/rules.rs"));
}

There are two warnings that are not fixed by this PR:

  • all the Token variants ending with Token themselves (Token::TagToken, Token::EOFToken...). I didn't fix this because these variants are used directly without the enum name (TagToken, EOFToken...). Not a fan, but outside of the scope of this PR
  • Some enum variants are never constructed, I've ignored this in simonwuelker@86dcdeb since I'm not sure if these can actually be removed

I've made individual commits for each "kind" of lint being addressed - I can squash these together if preferred.

simonwuelker and others added 16 commits August 29, 2024 23:33
Signed-off-by: Simon Wülker <[email protected]>
Signed-off-by: Simon Wülker <[email protected]>
This addresses the clippy::unreachable_pub lint
Signed-off-by: Simon Wülker <[email protected]>
This fixes a clippy lint

Signed-off-by: Simon Wülker <[email protected]>
These can all either use if-let branches
or the matches! macro

Signed-off-by: Simon Wülker <[email protected]>
Signed-off-by: Simon Wülker <[email protected]>
Signed-off-by: Simon Wülker <[email protected]>
Signed-off-by: Simon Wülker <[email protected]>
This also fixes a clippy warning

Signed-off-by: Simon Wülker <[email protected]>
This is necessary to allow html5ever to build (due
to #[deny(warnings)].

We might want to remove these in the future,
but I'm not sure whether they're necessary.
Copy link
Member

@mrobinson mrobinson left a comment

Choose a reason for hiding this comment

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

Nice!

@mrobinson mrobinson added this pull request to the merge queue Aug 31, 2024
Merged via the queue into servo:main with commit bb62eac Aug 31, 2024
5 of 6 checks passed
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.

2 participants