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

Add comparison_to_empty #74

Open
0xLucqs opened this issue Sep 18, 2024 · 23 comments
Open

Add comparison_to_empty #74

0xLucqs opened this issue Sep 18, 2024 · 23 comments
Labels
good first issue Good for newcomers lint

Comments

@0xLucqs
Copy link
Contributor

0xLucqs commented Sep 18, 2024

What it does

Checks for comparing to an empty slice such as [], and suggests using .is_empty() where applicable.

Why is this bad?

Some structures can answer .is_empty() much faster than checking for equality. So it is good to get into the habit of using .is_empty(), and having it is cheap. Besides, it makes the intent clearer than a manual comparison in some contexts.

Example

if s == ArrayTrait::new() {
    ..
}

Use instead:

if arr.is_empty() {
    ..
}
@0xLucqs 0xLucqs added good first issue Good for newcomers lint labels Sep 18, 2024
@0xLucqs
Copy link
Contributor Author

0xLucqs commented Sep 18, 2024

hey @Benjtalkshow wanna try this one ?

Copy link

onlydustapp bot commented Sep 18, 2024

Hey @SridharVadla45!
Thanks for showing interest.
We've created an application for you to contribute to Cairo lint.
Go check it out on OnlyDust!

@0xLucqs
Copy link
Contributor Author

0xLucqs commented Sep 18, 2024

sure any idea on how to proceed ?

@BernalHQ
Copy link
Contributor

Hi @0xLucqs, can i try this.

first i m going to identify which structs in Cairo support is_empty. Once identified, I’ll implement the logic to catch those cases and suggest fixes. Lastly, I’ll write some tests.

Copy link

onlydustapp bot commented Sep 18, 2024

Hey @BernalHQ!
Thanks for showing interest.
We've created an application for you to contribute to Cairo lint.
Go check it out on OnlyDust!

@0xLucqs
Copy link
Contributor Author

0xLucqs commented Sep 18, 2024

Hey @BernalHQ can you pick another issue plz (or if @SridharVadla45 doesn’t answer in the coming days you can take it)

@BernalHQ
Copy link
Contributor

Ok, i going to looking another issue, and i can do It, if dont answer It.

@jcampos2907
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Ive been leaning rust for the past year and looking to get a good first issue

@lauchaves
Copy link
Contributor

Hey @0xLucqs is this one available? If so, may I work on this! :D

Copy link

onlydustapp bot commented Sep 21, 2024

Hey @lauchaves!
Thanks for showing interest.
We've created an application for you to contribute to Cairo lint.
Go check it out on OnlyDust!

@MariangelaNM
Copy link
Contributor

MariangelaNM commented Sep 21, 2024

I am applying to this issue via OnlyDust platform.

Hi @0xLucqs, is there an opportunity for me to work on this project again? I previously worked on it and would love to contribute further.

@JoE11-y
Copy link
Contributor

JoE11-y commented Sep 23, 2024

Hey @0xLucqs, I’d love to take a look at this too.

@augustin-v
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I am an enthusiastic self-taught developer, that learnt Rust as my first language. With hands-on experience optimizing gas usage in projects like Node Guardians exercises (thinking in cairo, brainfvckvm). Which helped me learn how to write optimized code

How I plan on tackling this issue

As I have already reviewed the codebase, I would first create a linting rule that flags these comparisons and suggests using .is_empty() instead, and then test to ensure the rule correctly identifies and suggests improvements.

@od-hunter
Copy link

Hi @0xLucqs , please can I be assigned please? This would be my first time contributing to this repo and I’d love to be given the opportunity.

Copy link

onlydustapp bot commented Sep 24, 2024

Hey @od-hunter!
Thanks for showing interest.
We've created an application for you to contribute to Cairo lint.
Go check it out on OnlyDust!

@Benjtalkshow
Copy link

hey @Benjtalkshow wanna try this one ?

Hey @0xLucqs
I can handle the issue. I wanna try it

@0xLucqs
Copy link
Contributor Author

0xLucqs commented Sep 30, 2024

did anyone start anything ? In case no i this issue will be paused

@Benjtalkshow
Copy link

did anyone start anything ? In case no i this issue will be paused

Hey @0xLucqs ,
I already started working on it. Should i continue?

@0xLucqs
Copy link
Contributor Author

0xLucqs commented Sep 30, 2024

can you open a pr to show me what you have ?

@SumitGoulikar
Copy link

Hi @0xLucqs , can I be assigned please? This would be my first time contributing to this repository and I would love to be given this opportunity.

@Benjtalkshow
Copy link

can you open a pr to show me what you have ?

Alright

@Benjtalkshow
Copy link

Hello @0xLucqs , I just made a draft PR regarding this issue.
I will appreciate it if this issue is assigned to me. Thanks

@vic-Gray
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have experience optimizing Rust code for performance and clarity, focusing on efficient and maintainable solutions to address challenges.

How I plan on tackling this issue

The check compares a slice to an empty one, recommending .is_empty() for better performance and clarity. I'd replace manual comparisons with .is_empty() to improve the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers lint
Projects
None yet
Development

Successfully merging a pull request may close this issue.