Change defaults of accept-comment-above-statement and accept-comment-above-attributes

[tgross35]

This patch sets the two configuration options for undocumented_unsafe_blocks to true by default: these are accept-comment-above-statement and accept-comment-above-attributes. Having these values false by default prevents what many users would consider clean code, e.g. placing the // SAFETY: comment above a single-line functino call, rather than directly next to the argument.

This was originally discussed in #11162

changelog: [undocumented_unsafe_blocks]: set
accept-comment-above-statement and accept-comment-above-attributes to true by default.

[rustbot]

r? @giraffate

(rustbot has picked a reviewer for you, use r? to override)

[bors]

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

[tgross35]

Conflicts should be resolved

Since you were involved in the original conversation:

r? @Centri3

[Centri3]

LGTM, though we should probably use revisions for the default/custom configuration options instead of duplicating the file with minor comment changes, but that's not a big deal

Ideally however, these wouldn't even be configuration options as I highly doubt there are any cases (other than bugs) where somebody would actually want to have these set to false. We could probably deprecate them and say that this behavior will be forced in the future, but that can be done in a future PR.

@bors r+

[bors]

📌 Commit 90c37e1 has been approved by Centri3

It is now in the queue for this repository.

[bors]

⌛ Testing commit 90c37e1 with merge 07dc32a...

[bors]

💔 Test failed - checks-action_test

[tgross35]

LGTM, though we should probably use revisions for the default/custom configuration options instead of duplicating the file with minor comment changes, but that's not a big deal

If this is something that we can do now, is there an example? I have to fix something here anyway it seems so might as well change that too if possible

[Centri3]

You usually put the following headers at the top of the file:

//@revisions: a b
//@[a] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/mylint/a
//@[b] rustc-env:CLIPPY_CONF_DIR=tests/ui-toml/mylint/b

With a clippy.toml file existing in both subdirectories a and b. Doubt you can conditionally add a comment though so just have something like SAFETY: will be allowed if <configoption> is true

[bors]

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

[Centri3]

Hey @tgross35, any progress on the revisions? If not, this just needs a cargo collect-metadata run and a rebase and this can be merged.

[tgross35]

@Centri3 sorry about that, it fell off my radar. I think I got the dual config working right as you suggested, and did the rebase.

I'm not sure why github shows the four deleted/renamed files as conflicts when my tree is clean, I am trying to figure this out but do you have any ideas?

[tgross35]

I guess I was just rebasing against my own fork instead of upstream 🤦 I think this should be good to go

[Centri3]

Thanks!

@bors r+

[bors]

📌 Commit 1b3e5dd has been approved by Centri3

It is now in the queue for this repository.

[bors]

⌛ Testing commit 1b3e5dd with merge ddbe110...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Centri3
Pushing ddbe110 to master...