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 bool_assert_comparison #82

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

Add bool_assert_comparison #82

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

Comments

@0xLucqs
Copy link
Contributor

0xLucqs commented Sep 18, 2024

What it does

This lint warns about boolean comparisons in assert-like macros.

Why is this bad?

It is shorter to use the equivalent.

Example

assert_eq!("a".is_empty(), false);
assert_ne!("a".is_empty(), true);

Use instead:

assert!(!"a".is_empty());
@0xLucqs 0xLucqs added good first issue Good for newcomers lint labels Sep 18, 2024
@Josue19-08
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hello @0xLucqs, I would like to participate in this issue. I am part of Dojo Coding and have been working with this language. I would be happy to contribute to this project. I am familiar with languages such as Java, C#, .NET, C++, JS, and TS.

How I plan on tackling this issue

I would replace the boolean comparisons in assert! macros with direct assertions, simplifying the code and making it more readable. Then, I would test the changes to ensure everything works as intended.

@MariangelaNM
Copy link
Contributor

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hi, I'm Mariángela, a software developer with a passion for innovative solutions. I'm excited about the opportunity to contribute to this project and collaborate with the team.

How I plan on tackling this issue

Replace assert_eq! and assert_ne! with assert! and its negated form as appropriate and review codebase to ensure that assertions are written in a more concise and idiomatic manner.

@0xLucqs
Copy link
Contributor Author

0xLucqs commented Sep 18, 2024

@Josue19-08 the floor is yours. Don’t hesitate to ping me if you need any help

@Josue19-08
Copy link

Thanks @0xLucqs, excited to get started

@Manush-2005
Copy link

Hey @0xLucqs, I would like to take this issue up and contribute to cario lint from onlydust

@0xLucqs
Copy link
Contributor Author

0xLucqs commented Sep 21, 2024

@Manush-2005 it's already assigned i'll try to find something else for you but i'm afraid you didn't really understand the goal of this project

@Manush-2005
Copy link

Hey @0xLucqs , sorry for raising an PR without understanding the core problem. I am closing the PR I rasied and finding another issue and will solve it using your help.
Thanks for the reply

@mkaput mkaput added this to cairo-lint Dec 9, 2024
@github-project-automation github-project-automation bot moved this to Triage in cairo-lint Dec 9, 2024
@mkaput mkaput moved this from Triage to Backlog in cairo-lint Dec 11, 2024
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
Status: Backlog
Development

Successfully merging a pull request may close this issue.

4 participants