Skip to content

setup typos check in CI #134006

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

Merged
merged 1 commit into from
Jul 3, 2025
Merged

setup typos check in CI #134006

merged 1 commit into from
Jul 3, 2025

Conversation

klensy
Copy link
Contributor

@klensy klensy commented Dec 7, 2024

This allows to check typos in CI, currently for compiler only (to reduce commit size with fixes). With current setup, exclude list is quite short, so it worth trying?

Also includes commits with actual typo fixes.

MCP: rust-lang/compiler-team#817

typos check currently turned for:

  • ./compiler
  • ./library
  • ./src/bootstrap
  • ./src/librustdoc

After merging, PRs which enables checks for other crates (tools) can be implemented too.

Found typos will not break other jobs immediately: (tests, building compiler for perf run). Job will be marked as red on completion in ~ 20 secs, so you will not forget to fix it whenever you want, before merging pr.

Check typos: python x.py test tidy --extra-checks=spellcheck
Apply typo fixes: python x.py test tidy --extra-checks=spellcheck:fix (in case if there only 1 suggestion of each typo)

Current fail in this pr is expected and shows how typo errors emitted. Commit with error will be removed after r+.

@rustbot
Copy link
Collaborator

rustbot commented Dec 7, 2024

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Dec 7, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 7, 2024

Some changes occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Some changes occurred in match checking

cc @Nadrieril

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rust-log-analyzer

This comment has been minimized.

@klensy klensy force-pushed the typos branch 2 times, most recently from f01ec3c to 5b7daa7 Compare December 7, 2024 12:21
@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Dec 7, 2024

The Miri subtree was changed

cc @rust-lang/miri

@@ -795,7 +795,7 @@ impl<'a> Parser<'a> {
// When there are a few keywords in the last ten elements of `self.expected_tokens` and the current
// token is an identifier, it's probably a misspelled keyword.
// This handles code like `async Move {}`, misspelled `if` in match guard, misspelled `else` in `if`-`else`
// and mispelled `where` in a where clause.
// and misspelled `where` in a where clause.
Copy link
Member

Choose a reason for hiding this comment

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

they misspelled "misspelled" 😀

//! Note 2: this only applies to matcheable values. For example a value of type `Rc<u64>` can't be
//! Note 2: this only applies to matchable values. For example a value of type `Rc<u64>` can't be
Copy link
Member

@Nadrieril Nadrieril Dec 7, 2024

Choose a reason for hiding this comment

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

Didn't you add an exception for this in typos.toml? I don't know which is correct but if there's an exception this should be kept, presumably.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't seem to be reverted. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here #134006 (comment) reviewer said that this is not valid word, so i removed exception somewhere here 199e4e7.

@Nadrieril
Copy link
Member

I seem to recall a previous attempt at having typo-checks in CI that had a ton of false positives, this is much nicer, thanks. I can't approve the CI addition myself (does this require an FCP maybe?), but I looked through the typo fixes and they all look good to me.

@rust-log-analyzer

This comment has been minimized.

@rustbot rustbot added the A-meta Area: Issues & PRs about the rust-lang/rust repository itself label Dec 7, 2024
@lqd
Copy link
Member

lqd commented Dec 7, 2024

does this require an FCP maybe?

maybe an MCP?

Comment on lines +10 to +18
# Add exclusions here, lines should be like `x = "x"`, where `x` is excluded word.
#
# Also see docs: https://github.com/crate-ci/typos/blob/v1.28.2/docs/reference.md
Copy link
Member

Choose a reason for hiding this comment

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

Based on the documentation these aren't exclusions, they're custom additions to the dictionary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but it is one of the ways to silence linter, so i wrote as exclusions, to simplify wording.

@rustbot rustbot added O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface labels Dec 7, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 7, 2024

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @rust-lang/wg-const-eval

Some changes occurred to the platform-builtins intrinsics. Make sure the
LLVM backend as well as portable-simd gets adapted for the changes.

cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) F-autodiff `#![feature(autodiff)]` labels Jul 1, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 1, 2025

There are changes to the tidy tool.

cc @jieyouxu

Some changes occurred in compiler/rustc_codegen_llvm/src/llvm/enzyme_ffi.rs

cc @ZuseZ4

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

Some changes occurred in compiler/rustc_builtin_macros/src/autodiff.rs

cc @ZuseZ4

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

@@ -264,7 +264,7 @@ static mut TYPE_DESCRIPTOR: _TypeDescriptor = _TypeDescriptor {
// runtime under a try/catch block and the panic that we generate here will be
// used as the result of the exception copy. This is used by the C++ runtime to
// support capturing exceptions with std::exception_ptr, which we can't support
// because Box<dyn Any> isn't clonable.
// because Box<dyn Any> isn't cloneable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Both "clonable" and "cloneable" are valid, so this should be reverted.

"typos",
"spellcheck file checks",
// sync version with .github/workflows/ci.yml
Some("install tool via `cargo install [email protected]`".to_owned()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it have to be v1.34.0? Would it make more sense to omit the version number? Seems like "the latest version" would be better.

Copy link
Contributor

Choose a reason for hiding this comment

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

And s/ci.yml/spellcheck.yml/, as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added commit with check that ensures that correct version is installed. In case if version differs, output from local check and CI will diverge (which is bad). This should probably be not fatal error in case if people have different version for whatever case.

uses: actions/checkout@v4

- name: check typos
# sync version with src/tools/tidy/src/ext_tool_checks.rs
Copy link
Contributor

Choose a reason for hiding this comment

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

There are two locations in that file that need to be synced, please mention that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@nnethercote
Copy link
Contributor

Thanks for your patience on this, I think it's very close to being ready. That's a lot of nice typo fixes.

  • Please address my comments above.
  • The "don't merge" commit with the "outter" typo can now be removed.
  • Please squash all the other commits; I think this one is simpler to understand as a single commit.

And just to confirm my understanding:

  • This will run by default on CI, as a separate "Spellcheck" check. (Seen to deliberately fail above.)
  • It won't run in local x test tidy runs by default, because people might not have typos-cli installed.
  • You can run it locally with x test tidy --extra-checks=spellcheck if you have typos-cli installed.

@bors
Copy link
Collaborator

bors commented Jul 2, 2025

☔ The latest upstream changes (presumably #142974) made this pull request unmergeable. Please resolve the merge conflicts.

@klensy
Copy link
Contributor Author

klensy commented Jul 2, 2025

Squashed previous commits and added review commit + commit where required version of typos checked.

This will run by default on CI, as a separate "Spellcheck" check. (Seen to deliberately fail above.)

Yes, run by default by separate job and will not immediately stop any other jobs in case if spellcheck fails.

It won't run in local x test tidy runs by default, because people might not have typos-cli installed.

Nope, running x test tidy without --extra-checks=spellcheck or --extra-checks=spellcheck:fix will not call spellchecker.

You can run it locally with x test tidy --extra-checks=spellcheck if you have typos-cli installed.

Yes, and and can fix typos if x test tidy --extra-checks=spellcheck:fix used (if there no multiple suggestions).

@nnethercote
Copy link
Contributor

Looks good! Squash the final two commits into the first commit and r=me. Thanks.

@bors delegate=klensy

@bors
Copy link
Collaborator

bors commented Jul 2, 2025

✌️ @klensy, you can now approve this pull request!

If @nnethercote told you to "r=me" after making some further change, please make that change, then do @bors r=@nnethercote

@klensy
Copy link
Contributor Author

klensy commented Jul 3, 2025

@bors r=@nnethercote

@bors
Copy link
Collaborator

bors commented Jul 3, 2025

📌 Commit c76d032 has been approved by nnethercote

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 3, 2025
jdonszelmann added a commit to jdonszelmann/rust that referenced this pull request Jul 3, 2025
setup typos check in CI

This allows to check typos in CI, currently for compiler only (to reduce commit size with fixes). With current setup, exclude list is quite short, so it worth trying?

Also includes commits with actual typo fixes.

MCP: rust-lang/compiler-team#817

typos check currently turned for:
* ./compiler
* ./library
* ./src/bootstrap
* ./src/librustdoc

After merging, PRs which enables checks for other crates (tools) can be implemented too.

Found typos will **not break** other jobs immediately: (tests, building compiler for perf run). Job will be marked as red on completion in ~ 20 secs, so you will not forget to fix it whenever you want, before merging pr.

Check typos: `python x.py test tidy --extra-checks=spellcheck`
Apply typo fixes: `python x.py test tidy --extra-checks=spellcheck:fix` (in case if there only 1 suggestion of each typo)

Current fail in this pr is expected and shows how typo errors emitted. Commit with error will be removed after r+.
bors added a commit that referenced this pull request Jul 3, 2025
Rollup of 6 pull requests

Successful merges:

 - #134006 (setup typos check in CI)
 - #142876 (Port `#[target_feature]` to new attribute parsing infrastructure)
 - #143038 (avoid suggesting traits from private dependencies)
 - #143083 (Fix rustdoc not correctly showing attributes on re-exports)
 - #143283 (document optional jobs)
 - #143329 (minicore: use core's `diagnostic::on_unimplemented` messages)

Failed merges:

 - #143237 (Port `#[no_implicit_prelude]` to the new attribute parsing infrastructure)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f6d37a2 into rust-lang:master Jul 3, 2025
11 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 3, 2025
rust-timer added a commit that referenced this pull request Jul 3, 2025
Rollup merge of #134006 - klensy:typos, r=nnethercote

setup typos check in CI

This allows to check typos in CI, currently for compiler only (to reduce commit size with fixes). With current setup, exclude list is quite short, so it worth trying?

Also includes commits with actual typo fixes.

MCP: rust-lang/compiler-team#817

typos check currently turned for:
* ./compiler
* ./library
* ./src/bootstrap
* ./src/librustdoc

After merging, PRs which enables checks for other crates (tools) can be implemented too.

Found typos will **not break** other jobs immediately: (tests, building compiler for perf run). Job will be marked as red on completion in ~ 20 secs, so you will not forget to fix it whenever you want, before merging pr.

Check typos: `python x.py test tidy --extra-checks=spellcheck`
Apply typo fixes: `python x.py test tidy --extra-checks=spellcheck:fix` (in case if there only 1 suggestion of each typo)

Current fail in this pr is expected and shows how typo errors emitted. Commit with error will be removed after r+.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-testsuite Area: The testsuite used to check the correctness of rustc A-tidy Area: The tidy tool F-autodiff `#![feature(autodiff)]` O-unix Operating system: Unix-like O-wasi Operating system: Wasi, Webassembly System Interface S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. S-waiting-on-MCP Status: PR has a compiler MCP and is waiting for the compiler MCP to complete. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.