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

rule: Fix pointless reassign issue when using with #907

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

charlieegan3
Copy link
Member

@charlieegan3 charlieegan3 commented Jul 9, 2024

Fixes #904

I find the times to be comparable for the regal bundle + a simple violating file.

regal $ hyperfine --warmup 3 './regal-main lint . --disable-all --enable=pointless-reassignment' './regal-pointless lint .
 --disable-all --enable=pointless-reassignment'
Benchmark 1: ./regal-main lint . --disable-all --enable=pointless-reassignment
  Time (mean ± σ):      1.171 s ±  0.077 s    [User: 6.188 s, System: 0.192 s]
  Range (min … max):    1.108 s …  1.370 s    10 runs
 
Benchmark 2: ./regal-pointless lint . --disable-all --enable=pointless-reassignment
  Time (mean ± σ):     665.8 ms ±  11.3 ms    [User: 2891.0 ms, System: 100.2 ms]
  Range (min … max):   644.2 ms … 688.2 ms    10 runs
 
Summary
  ./regal-pointless lint . --disable-all --enable=pointless-reassignment ran
    1.76 ± 0.12 times faster than ./regal-main lint . --disable-all --enable=pointless-reassignment

Fixes StyraInc#904

I find the times to be comparable for the regal bundle +
a simple violating file.

```sh
regal $ go build && time ./regal lint .  --disable-all --enable=pointless-r
eassignment
Rule:           pointless-reassignment
Description:    Pointless reassignment of variable
Category:       style
Location:       thing.rego:10:6
Text:           foo := e with foo as 2
Documentation:  https://docs.styra.com/regal/rules/style/pointless-reassignment

201 files linted. 1 violation found in 1 file.

real    0m1.906s
user    0m6.344s
sys     0m0.244s
[3]regal $ bran
Current:  main
Switched to branch 'pointless'
Your branch is ahead of 'main' by 1 commit.
  (use "git push" to publish your local commits)
regal $ go build && time ./regal lint .  --disable-all --enable=pointless-reassignment
201 files linted. No violations found.

real    0m1.580s
user    0m2.930s
sys     0m0.109s
```

Signed-off-by: Charlie Egan <[email protected]>
@charlieegan3 charlieegan3 merged commit 288d9c3 into StyraInc:main Jul 9, 2024
3 checks passed
@charlieegan3
Copy link
Member Author

Thanks Stephan

@charlieegan3 charlieegan3 deleted the pointless branch July 9, 2024 16:27
@anderseknert
Copy link
Member

Good catch! And a good fix. Thanks! 👏

srenatus pushed a commit to srenatus/regal that referenced this pull request Oct 1, 2024
Fixes StyraInc#904

I find the times to be comparable for the regal bundle +
a simple violating file.

```sh
regal $ go build && time ./regal lint .  --disable-all --enable=pointless-r
eassignment
Rule:           pointless-reassignment
Description:    Pointless reassignment of variable
Category:       style
Location:       thing.rego:10:6
Text:           foo := e with foo as 2
Documentation:  https://docs.styra.com/regal/rules/style/pointless-reassignment

201 files linted. 1 violation found in 1 file.

real    0m1.906s
user    0m6.344s
sys     0m0.244s
[3]regal $ bran
Current:  main
Switched to branch 'pointless'
Your branch is ahead of 'main' by 1 commit.
  (use "git push" to publish your local commits)
regal $ go build && time ./regal lint .  --disable-all --enable=pointless-reassignment
201 files linted. No violations found.

real    0m1.580s
user    0m2.930s
sys     0m0.109s
```

Signed-off-by: Charlie Egan <[email protected]>
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.

bug: pointless reassignment should ignore with
3 participants