Skip to content

Change lints hyphen to underscore #308

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 2 commits into from
May 26, 2022
Merged

Change lints hyphen to underscore #308

merged 2 commits into from
May 26, 2022

Conversation

hafeoz
Copy link
Contributor

@hafeoz hafeoz commented May 26, 2022

rustc -D nonstandard-style is a valid syntax; #[deny(nonstandard_style)] is also valid. However, #[deny(nonstandard-style)] will cause:

error: expected one of `(`, `,`, `::`, or `=`, found `-`
  --> example/src/lib.rs:12:19
   |
12 | #[deny(nonstandard-style)]
   |                   ^ expected one of `(`, `,`, `::`, or `=`

Which is confusing if someone is trying to copy-paste the code into their own.

@hafeoz hafeoz changed the title Change lints hyphen underscore Change lints hyphen to underscore May 26, 2022
@marcoieni
Copy link
Collaborator

Great, thanks 😄
We could also remove the ,ignore flag by adding an fn main {} at the end of the code block.
So that this code block is tested when running mdbook test in the CI.

Here is an example:

```rust
#[deny(bad_style,
       const_err,
       dead_code,
       improper_ctypes,
       non_shorthand_field_patterns,
       no_mangle_generic_items,
       overflowing_literals,
       path_statements,
       patterns_in_fns_without_body,
       private_in_public,
       unconditional_recursion,
       unused,
       unused_allocation,
       unused_comparisons,
       unused_parens,
       while_true)]
fn main() {}

What do you think? :)

@hafeoz
Copy link
Contributor Author

hafeoz commented May 26, 2022

Great, thanks smile We could also remove the ,ignore flag by adding an fn main {} at the end of the code block. So that this code block is tested when running mdbook test in the CI.

Here is an example:

```rust
#[deny(bad_style,
       const_err,
       dead_code,
       improper_ctypes,
       non_shorthand_field_patterns,
       no_mangle_generic_items,
       overflowing_literals,
       path_statements,
       patterns_in_fns_without_body,
       private_in_public,
       unconditional_recursion,
       unused,
       unused_allocation,
       unused_comparisons,
       unused_parens,
       while_true)]
fn main() {}

What do you think? :)

How about marking the attributes as inner attributes? LIke

#![deny(bad_style, ...)]

By doing this way, we won't need fn main() {} in the code.

@marcoieni
Copy link
Collaborator

Oh, that's even better, yes!

@hafeoz
Copy link
Contributor Author

hafeoz commented May 26, 2022

Great, thanks smile We could also remove the ,ignore flag by adding an fn main {} at the end of the code block. So that this code block is tested when running mdbook test in the CI.
Here is an example:

```rust
#[deny(bad_style,
       const_err,
       dead_code,
       improper_ctypes,
       non_shorthand_field_patterns,
       no_mangle_generic_items,
       overflowing_literals,
       path_statements,
       patterns_in_fns_without_body,
       private_in_public,
       unconditional_recursion,
       unused,
       unused_allocation,
       unused_comparisons,
       unused_parens,
       while_true)]
fn main() {}

What do you think? :)

How about marking the attributes as inner attributes? LIke

#![deny(bad_style, ...)]

By doing this way, we won't need fn main() {} in the code.

Currently this modification fails to build due to rust-lang/rust#97440

@marcoieni
Copy link
Collaborator

marcoieni commented May 26, 2022

ok, what do you prefer to do, then? All the proposed solutions work for me. The PR can even be merged as it is now.

@hafeoz
Copy link
Contributor Author

hafeoz commented May 26, 2022

ok, what do you prefer to do, then? For me, we can also merge this PR as it is.

I'm currently preparing for a PR to fix the ICE. In the meanwhile, maybe we should continue to mark the code block as ignore until the PR is merged?

marcoieni
marcoieni previously approved these changes May 26, 2022
Copy link
Collaborator

@marcoieni marcoieni left a comment

Choose a reason for hiding this comment

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

Sounds good to me. Should we merge this? 🚀

@hafeoz
Copy link
Contributor Author

hafeoz commented May 26, 2022

Sounds good to me. Should we merge this? rocket

I have changed lints to inner attributes👍 I think it's ready now

Copy link
Collaborator

@marcoieni marcoieni left a comment

Choose a reason for hiding this comment

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

Thanks!

@marcoieni marcoieni enabled auto-merge (squash) May 26, 2022 21:59
@marcoieni marcoieni merged commit 662a519 into rust-unofficial:main May 26, 2022
@hafeoz hafeoz deleted the patch-1 branch May 26, 2022 22:01
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