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

Detect Enum Objects with empty attribute body using {} #12007

Closed
jetlime opened this issue Dec 24, 2023 · 7 comments · Fixed by #12047
Closed

Detect Enum Objects with empty attribute body using {} #12007

jetlime opened this issue Dec 24, 2023 · 7 comments · Fixed by #12047
Assignees
Labels
A-lint Area: New lints

Comments

@jetlime
Copy link
Contributor

jetlime commented Dec 24, 2023

What it does

The proposed lint feature aims to improve code readability and consistency by discouraging the use of empty attribute objects {} succeeding structures without attributes. In Rust, empty attribute objects are often used to represent variants of an enum that carry no additional information. However, this usage can be misleading and may result in less expressive and clear code.

This lint feature shall check for the presence of empty attribute objects {} immediately following enum variants without associated data. If such a pattern is detected, the lint will emit a warning. This warning encourages developers to reconsider the necessity of the empty attribute object and suggests using the enum variant directly without the unnecessary attribute object.

Advantage

  • Prevent Code Blow, simplify code.

Drawbacks

No response

Example

pub enum Decoder {
    Absent {},
    Symbol(u8)
}


#[derive(Debug, thiserror::Error)]
pub enum CustomError {
    #[error("This is a custom error without attributes.")]
    CustomError { },
}

type Result<T, E = CustomError> = std::result::Result<T, E>;

impl Decoder {
    /// # Errors
    /// 
    /// May return an error..
    pub fn decode(&mut self, input: u64) -> Result<u8> {
            if input > 1 { return Err(CustomError::CustomError { })};

            match self {
                Decoder::Symbol(symbol) => {
                    Ok(*symbol)
                }
                Decoder::Absent{} => {
                    Ok(b'i')
                }
            }
    }
}

Could be written as:

pub enum Decoder {
    Absent,
    Symbol(u8)
}

#[derive(Debug, thiserror::Error)]
pub enum CustomError {
    #[error("This is a custom error without attributes.")]
    CustomError,
}

type Result<T, E = CustomError> = std::result::Result<T, E>;

impl Decoder {
    /// # Errors
    /// 
    /// May return an error..
    pub fn decode(&mut self, input: u64) -> Result<u8> {
            if input > 1 { return Err(CustomError::CustomError)};

            match self {
                Decoder::Symbol(symbol) => {
                    Ok(*symbol)
                }
                Decoder::Absent => {
                    Ok(b'i')
                }
            }
    }
}

In both cases, the nursery and the pedantic mode of clippy do not output any warnings nor errors. To prevent code blow, structures without attributes shall not be allowed to be succeeded by an empty attribute object, {}. The example shall result in a warning, while the second one should not.

Tasks

No tasks being tracked yet.
@jetlime jetlime added the A-lint Area: New lints label Dec 24, 2023
@jetlime jetlime changed the title Detect Empty Attribute Objects {} Detect Objects with empty attribute body using {} Dec 27, 2023
@ARandomDev99
Copy link
Contributor

@rustbot claim

@blyxyas
Copy link
Member

blyxyas commented Dec 28, 2023

This is already handled by empty_structs_with_brackets ฅ*•ω•*ฅ♡

EDIT: It seems like it doesn't, it just focuses on structs. I think it'd be better to mix the current open PR with that lint instead of making a whole new lint. (Maybe with a name change to the current lint)

@blyxyas blyxyas closed this as completed Dec 28, 2023
@blyxyas blyxyas reopened this Dec 28, 2023
@jetlime jetlime changed the title Detect Objects with empty attribute body using {} Detect Enum Objects with empty attribute body using {} Dec 28, 2023
@ARandomDev99
Copy link
Contributor

A new lint called brackets_without_fields that does the work of both empty_structs_with_brackets and the lint added in the PR would do the trick. However, I can't find the guidelines in the Clippy book on how to proceed while moving a lint. Can someone please point me in the right direction?

@m-rph
Copy link
Contributor

m-rph commented Dec 28, 2023

A merged lint would mean deprecating the previous one.

I think a better way to handle this is to have both lints in the same file with a sufficiently descriptive name, and issuing the right one.

This pattern already exists in a few places, so it's established.

@Xuanwo
Copy link

Xuanwo commented Dec 29, 2023

Changing

pub enum Decoder {
    Absent {},
    Symbol(u8)
}

into

pub enum Decoder {
    Absent,
    Symbol(u8)
}

is a breaking change.

Can we determine if the user intends to do this? Also, we're not enabling this lint by default, correct?

@ARandomDev99
Copy link
Contributor

@Xuanwo

Also, we're not enabling this lint by default, correct?

The lint would be placed in the "Restriction" Group which seems to have lints that are allow by default.

@ARandomDev99
Copy link
Contributor

I have opened a new PR incorporating the changes suggested by @partiallytyped.

@bors bors closed this as completed in 7f185bd Dec 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
5 participants