Skip to content

enable the dbg_macro clippy rule - #4821

Closed
DetachHead wants to merge 1 commit into
facebook:mainfrom
DetachHead:clippy-ban-dbg-macro
Closed

enable the dbg_macro clippy rule#4821
DetachHead wants to merge 1 commit into
facebook:mainfrom
DetachHead:clippy-ban-dbg-macro

Conversation

@DetachHead

Copy link
Copy Markdown
Contributor

Summary

after @connernilsen pointed out that i accidentally committed a dbg! macro, i wondered if there was a clippy rule to prevent such mistakes (see #4729 (comment)). turns out there is!

note that enabling a rule globally seems to be nowhere near as straightforward as it is in most other linters, so i had to make some other changes:

Test Plan

intentionally inserted a dbg! macro and ran cargo clippy:

error: the `dbg!` macro is intended as a debugging tool
  --> crates/pyrefly_config/src/migration/error_codes.rs:72:9
   |
72 |         dbg!(&pyright_cfg.type_checking_mode);
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.98.0/index.html#dbg_macro
   = note: requested on the command line with `-D clippy::dbg-macro`
help: remove the invocation before committing it to a version control system
   |
72 -         dbg!(&pyright_cfg.type_checking_mode);
72 +         &pyright_cfg.type_checking_mode;
   |

error: could not compile `pyrefly_config` (lib) due to 1 previous error

@meta-codesync

meta-codesync Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

@stroxler has imported this pull request. If you are a Meta employee, you can view this in D118959745. (Because this pull request was imported automatically, there will not be any future comments.)

@stroxler stroxler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

Don't worry about the red internal signal, I had to patch this because cargo config is derived from our internal buck build setup but the change is good. I'll try to get this in soon, merging requires a second review and it's a holiday weekend so I'm not sure how soon

@samwgoldman samwgoldman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

@meta-codesync meta-codesync Bot closed this in 4208c19 Sep 5, 2026
@meta-codesync meta-codesync Bot added the Merged label Sep 5, 2026
@meta-codesync

meta-codesync Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

@stroxler merged this pull request in 4208c19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants