Skip to content
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

Design decisions around the #[expect] attribute #191

Closed
xFrednet opened this issue Jan 12, 2023 · 6 comments
Closed

Design decisions around the #[expect] attribute #191

xFrednet opened this issue Jan 12, 2023 · 6 comments
Labels
meeting-proposal Proposal for a lang team design meeting meeting-scheduled Lang team design meeting that has a scheduled date T-lang

Comments

@xFrednet
Copy link
Member

xFrednet commented Jan 12, 2023

Summary

The #[expect] attribute as proposed in RFC 2383 adds a new attribute that suppresses lint emissions, but issues a warning if the expected lint doesn't trigger anymore. In the proposed RFC it was described as this:

This RFC adds an expect lint attribute that functions identically to allow, but will cause a lint to be emitted when the code it decorates does not raise a lint warning.

A working implementation of the attribute is available in nightly with the lint_reasons feature. See this playground as an example. The goal of this meeting is to resolve the last questions remaining in the tracking issue.

Questions

The questions also contain a collapsed explanation of the current behavior. (This might not work on the mobile view)

1. Should the attribute really be called #[expect] or is the name too generic? src

2. How should the #[expect] attribute interact with other lint attributes, like allow, warn, deny? src

For example, should this code fulfill the expectation or not?

#![feature(lint_reasons)]

#[expect(unused_variables)]
pub fn f() {
    #[warn(unused_variables)]
    let x = 1;
}
Current implementation

Currently, expectations are only fulfilled by lint emissions directly affected by it. The example above would trigger two warnings, one for the unused value x and one for the fulfilled expectation.

3 Should an expectation be fulfilled, by a lint that would otherwise be allowed? src

For example, should this code fulfill the expectation or not?

#![feature(lint_reasons)]
#![allow(unused)]

#[allow(unsafe_code)]
pub fn f() {
    #[expect(unsafe_code)]
    unsafe {
    }
}
Current implementation

The current implementation only checks if a lint is emitted at a node affected by the lint attribute. The example above would therefore fulfill the expectation, just like #[warn] would cause the code to trigger the lint. If this wouldn't be the case, the #[expect] attribute would change behavior based on the outer lint level. This can become confusing if the user defines a lint level via console arguments like this cargo clippy -- -W clippy::pedantic

Background reading

Note

I'd like to attend the meeting as well. It would be awesome if I could be notified when the meeting has been scheduled. I'll also keep an eye on the calender :)

@nikomatsakis
Copy link
Contributor

Hi @xFrednet! Question from the planning meeting this month. Do you think you would have time to author a document for the meeting to guide discussion?

@xFrednet
Copy link
Member Author

xFrednet commented Feb 8, 2023

Hey @nikomatsakis, it depends on when the meeting is, but I'd be up for it :). Do you have a template or example for the document? I tried to write the issue in a way that already guides the discussion :)

@nikomatsakis
Copy link
Contributor

@xFrednet would March 8th work for you? Our meetings typically held at 13:00 US Eastern time (19:00 CEST). Regarding a template, I can send you some examples-- the key thing would be summarizing enough of the background material to have a self-contained discussion.

i.e., what are some of the proposed answers to those questions, and what are their pros/cons?

bonus points for taking an opinionated position about what you think is best :)

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Feb 8, 2023

@xFrednet we're going to take a stab at better doc guidelines, but here are a few docs from past meetings that were productive:

Key point is to identify clearly the topics to discuss (and, often, what not to discuss) and to give the background needed for that.

Note that the docs include minutes of the actual converations and questions that happened -- best thing is to author the doc in hackmd so we can live edit it during the meeting.

One particular danger is that sometimes docs get too in the weeds with details, where really what is needed from the lang team is more alignment around the high-level goals, or which design features should be prioritized (i.e., the question is not "do we do X or Y" it's "which use case is more important").

@xFrednet
Copy link
Member Author

xFrednet commented Feb 8, 2023

@nikomatsakis The 8th of March works for me! Thank you for the examples, I'll read through them and take a stab at it :)

@nikomatsakis nikomatsakis added the meeting-scheduled Lang team design meeting that has a scheduled date label Feb 21, 2023
@xFrednet
Copy link
Member Author

xFrednet commented Mar 1, 2023

Hey @nikomatsakis, I've created a document with some background information. The questions which should be discussed as well as the possible solutions. Here is a link:

https://hackmd.io/@xFrednet/rust-lang-team191

I've limited the write permissions for now, but will enable them for the meeting. If you have any comments, I'm happy to address them :)

@tmandry tmandry closed this as completed Apr 5, 2023
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 26, 2024
…y, r=Urgau,blyxyas

Let's `#[expect]` some lints: Stabilize `lint_reasons` (RFC 2383)

Let's give this another try! The [previous stabilization attempt](rust-lang#99063) was stalled by some unresolved questions. These have been discussed in a [lang team](rust-lang/lang-team#191) meeting. The last open question, regarding the semantics of the `#[expect]` attribute was decided on in rust-lang#115980

I've just updated the [stabilization report](rust-lang#54503 (comment)) with the discussed questions and decisions. Luckily, the decision is inline with the current implementation.

This hopefully covers everything. Let's hope that the CI will be green like the spring.

fixes rust-lang#115980
fixes rust-lang#54503

---

r? `@wesleywiser`

Tacking Issue: rust-lang#54503
Stabilization Report: rust-lang#54503 (comment)
Documentation Update: rust-lang/reference#1237

<!--
For Clippy:

changelog: [`allow_attributes`]: Is now available on stable, since the `lint_reasons` feature was stabilized
changelog: [`allow_attributes_without_reason`]: Is now available on stable, since the `lint_reasons` feature was stabilized
-->

---

Roses are red,
Violets are blue,
Let's expect lints,
With reason clues
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jun 27, 2024
…u,blyxyas

Let's `#[expect]` some lints: Stabilize `lint_reasons` (RFC 2383)

Let's give this another try! The [previous stabilization attempt](rust-lang/rust#99063) was stalled by some unresolved questions. These have been discussed in a [lang team](rust-lang/lang-team#191) meeting. The last open question, regarding the semantics of the `#[expect]` attribute was decided on in rust-lang/rust#115980

I've just updated the [stabilization report](rust-lang/rust#54503 (comment)) with the discussed questions and decisions. Luckily, the decision is inline with the current implementation.

This hopefully covers everything. Let's hope that the CI will be green like the spring.

fixes #115980
fixes #54503

---

r? `@wesleywiser`

Tacking Issue: rust-lang/rust#54503
Stabilization Report: rust-lang/rust#54503 (comment)
Documentation Update: rust-lang/reference#1237

<!--
For Clippy:

changelog: [`allow_attributes`]: Is now available on stable, since the `lint_reasons` feature was stabilized
changelog: [`allow_attributes_without_reason`]: Is now available on stable, since the `lint_reasons` feature was stabilized
-->

---

Roses are red,
Violets are blue,
Let's expect lints,
With reason clues
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jun 27, 2024
…u,blyxyas

Let's `#[expect]` some lints: Stabilize `lint_reasons` (RFC 2383)

Let's give this another try! The [previous stabilization attempt](rust-lang/rust#99063) was stalled by some unresolved questions. These have been discussed in a [lang team](rust-lang/lang-team#191) meeting. The last open question, regarding the semantics of the `#[expect]` attribute was decided on in rust-lang/rust#115980

I've just updated the [stabilization report](rust-lang/rust#54503 (comment)) with the discussed questions and decisions. Luckily, the decision is inline with the current implementation.

This hopefully covers everything. Let's hope that the CI will be green like the spring.

fixes #115980
fixes #54503

---

r? `@wesleywiser`

Tacking Issue: rust-lang/rust#54503
Stabilization Report: rust-lang/rust#54503 (comment)
Documentation Update: rust-lang/reference#1237

<!--
For Clippy:

changelog: [`allow_attributes`]: Is now available on stable, since the `lint_reasons` feature was stabilized
changelog: [`allow_attributes_without_reason`]: Is now available on stable, since the `lint_reasons` feature was stabilized
-->

---

Roses are red,
Violets are blue,
Let's expect lints,
With reason clues
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jun 28, 2024
…u,blyxyas

Let's `#[expect]` some lints: Stabilize `lint_reasons` (RFC 2383)

Let's give this another try! The [previous stabilization attempt](rust-lang/rust#99063) was stalled by some unresolved questions. These have been discussed in a [lang team](rust-lang/lang-team#191) meeting. The last open question, regarding the semantics of the `#[expect]` attribute was decided on in rust-lang/rust#115980

I've just updated the [stabilization report](rust-lang/rust#54503 (comment)) with the discussed questions and decisions. Luckily, the decision is inline with the current implementation.

This hopefully covers everything. Let's hope that the CI will be green like the spring.

fixes #115980
fixes #54503

---

r? `@wesleywiser`

Tacking Issue: rust-lang/rust#54503
Stabilization Report: rust-lang/rust#54503 (comment)
Documentation Update: rust-lang/reference#1237

<!--
For Clippy:

changelog: [`allow_attributes`]: Is now available on stable, since the `lint_reasons` feature was stabilized
changelog: [`allow_attributes_without_reason`]: Is now available on stable, since the `lint_reasons` feature was stabilized
-->

---

Roses are red,
Violets are blue,
Let's expect lints,
With reason clues
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Jul 11, 2024
…u,blyxyas

Let's `#[expect]` some lints: Stabilize `lint_reasons` (RFC 2383)

Let's give this another try! The [previous stabilization attempt](rust-lang/rust#99063) was stalled by some unresolved questions. These have been discussed in a [lang team](rust-lang/lang-team#191) meeting. The last open question, regarding the semantics of the `#[expect]` attribute was decided on in rust-lang/rust#115980

I've just updated the [stabilization report](rust-lang/rust#54503 (comment)) with the discussed questions and decisions. Luckily, the decision is inline with the current implementation.

This hopefully covers everything. Let's hope that the CI will be green like the spring.

fixes #115980
fixes #54503

---

r? `@wesleywiser`

Tacking Issue: rust-lang/rust#54503
Stabilization Report: rust-lang/rust#54503 (comment)
Documentation Update: rust-lang/reference#1237

<!--
For Clippy:

changelog: [`allow_attributes`]: Is now available on stable, since the `lint_reasons` feature was stabilized
changelog: [`allow_attributes_without_reason`]: Is now available on stable, since the `lint_reasons` feature was stabilized
-->

---

Roses are red,
Violets are blue,
Let's expect lints,
With reason clues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meeting-proposal Proposal for a lang team design meeting meeting-scheduled Lang team design meeting that has a scheduled date T-lang
Projects
Development

No branches or pull requests

3 participants