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

New rules for leading spaces of binary operators #263

Merged
merged 9 commits into from
Aug 9, 2023

Conversation

so298
Copy link
Contributor

@so298 so298 commented Jul 29, 2023

Hi, @dalance!

I added some new rules for leading spaces of binary operators.
The names of the new rules are the following:

  • style_operator_arithmetic_leading_space
  • style_operator_boolean_leading_space
  • style_operator_integer_leading_space

Each of these rules corresponds to the following existing rules.

  • style_operator_arithmetic
  • style_operator_boolean
  • style_operator_integer

These existing rules are for the number of following spaces and do not support leading spaces.

For example:

When I give the following SystemVerilog code to the linter with style_operator_arithmetic rule enabled,

module M;
  localparam int P2 = a  + b; // Multiple spaces before `+`.

  // No space before `*`.
  localparam int P3 = a* b;

endmodule

I expect the result that says the numbers of the leading spaces of * and + operators are not appropriate, but no message will be found actually because this existing rule do not consider leading space.

With the new rules I implemeted, this message will be found for the sample code.

Fail: style_operator_arithmetic_leading_space
   --> test.sv:2:26
  |
2 |   localparam int P2 = a  + b; // Multiple spaces before `+`.
  |                          ^ hint  : Put exact one space before binary arithmetic operators.
  |                            reason: Consistent use of whitespace enhances readability by reducing visual noise.

Fail: style_operator_arithmetic_leading_space
   --> test.sv:5:24
  |
5 |   localparam int P3 = a* b;
  |                        ^ hint  : Put exact one space before binary arithmetic operators.
  |                          reason: Consistent use of whitespace enhances readability by reducing visual noise.

The new rules only consider the number of leading spaces. (do not consider following spaces)

@DaveMcEwan
Copy link
Contributor

@so298 Nice one :) I notice a couple of little things.

  1. Can you add your name to the list of contributors (bottom of CONTRIBUTING.md) as part of this PR.
  2. Should these be enabled in the style ruleset? These new rules look like a great addition to me :)
  3. Minor spelling change in the hints, "exact" -> "exactly".

@so298
Copy link
Contributor Author

so298 commented Jul 30, 2023

@DaveMcEwan

Thank you for your comment!

I agree with the idea that these rules are enabled in the style ruleset.
I modified rulesets/style.toml in the last commit.

Also, I fixed spelling mistake and added my name to CONTRIBUTING.md.

pr263 Add the new `style_*_leading_space` rules to ruleset-style.
@so298
Copy link
Contributor Author

so298 commented Aug 2, 2023

@DaveMcEwan
Merged so298#1
Sorry for being late.

@dalance
Copy link
Owner

dalance commented Aug 9, 2023

Thank you for your contribution.
I'll merge this PR.

@dalance dalance merged commit 2b76fa6 into dalance:master Aug 9, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants