Added more lints

[lebensterben]

The list of new lints are:

• clippy::all
• clippy::pedantic
• absolute_paths_not_starting_with_crate
• rustdoc::invalid_html_tags
• missing_copy_implementations
• missing_debug_implementations
• semicolon_in_expressions_from_macros
• unreachable_pub
• unused_extern_crates
• variant_size_differences
• clippy::missing_const_for_fn

[untitaker]

I think this is fine. Explicitly documenting potential casting problems (overflow when casting to i32) is particularly good.

I think adding Debug impl is also good to have, I think what many of the added ones print is not great but we can improve that later. It's definetly annoying to not have any at all

I am unsure about pedantic. I ran clippy with pedantic a while back and didn't really like the results enough to fully commit to using it. Let's see how it goes

I don't agree with const fn lint on principle, I don't think we should add it

[lebensterben]

It's beneficial to derive Debug/Copy/Clone and marking functions as "const" when the library is still in an early age.

If later on someone needs to use those traits or use some functions in "const" context then that won't result in too many lines of changes.

[untitaker]

I agree that Debug should be there for every public type, but I don't agree about any of the other ones. implementing a trait for a public type or adding const to a public function is a commitment you make to users about your API, as removing the trait impl or removing const is a breaking change.

[lebensterben]

okay I will remove "clippy::missing_const_for_fn".

[lebensterben]

Updated as requested.

[untitaker]

hm those github actions warnings on the State enum are tricky (see diff view). after all this enum is usually pub(crate), but not if used in tests. so you get a warning when compiling without tests feature

can we silence the lint here?

the integration-tests feature was a mistake :( just don't have better ideas

[lebensterben]

the last commit is unnecessary.
you just need to add "--all-features" to "cargo clippy".

[lebensterben]

btw I don't understand the intention for "integration-tests" feature. even without them the dev-dependency won't be pulled in for library users.

[untitaker]

you just need to add "--all-features" to "cargo clippy".

clippy lints should pass either way

btw I don't understand the intention for "integration-tests" feature. even without them the dev-dependency won't be pulled in for library users.

it is a tricky situation that I don't know how to resolve. the html5lib_tokenizer tests need:

• access to private APIs
• custom test harness

unit tests can have private API access, and integration tests can have custom test harness. So my solution was to write integration test, and have a featureflag that, when enabled, exposes private APIs of html5gum publicly

[lebensterben]

what do you mean by "custom test harness"

[untitaker]

by defining harness = false in Cargo.toml, you can replace the entire test framework with your own program. I use https://github.com/LukasKalbertodt/libtest-mimic instead of the builtin testing framework, which allows neat things like declaring tests programmatically.

Since the actual testsuite is a folder of JSON files and each file contains multiple testcases, this allows me to walk that folder, parse the files, and generate testcases.

See https://github.com/untitaker/html5gum/blob/main/tests/html5lib_tokenizer.rs -- there's no #[test] usage here, just a main function. It still runs under cargo test

in case you think that's absolutely insane, in my defense html5ever does the same thing, they don't use libtest-mimic, instead they forked some internal rustc stuff and just... published it on crates.io (see rustc-test crate)

[lebensterben]

instead they forked some internal rustc stuff and just... published it on crates.io

I actually prefer this...

[lebensterben]

It's great if you can add some developer notes in readme explaining the "integration-tests" stuff.

[untitaker]

I actually prefer this...

it's still a custom test harness fwiw, just happens to be a fork of an old version of the standard test harness

libtest-mimic seems significantly smaller and attempts to look and feel the same, for the most part

[untitaker]

It's great if you can add some developer notes in readme explaining the "integration-tests" stuff.

Yes. If you have more questions/raised eyebrows please just file issues for them, I can probably explain most of it, I hope

[untitaker]

I have no idea why the codecov lint is still failing. I had hoped that my recent commits to master would've fixed that. I'll just merge.

Thank you very much! Again I really like the int-casting lint in particular