Skip to content

Conversation

@webwarrior-ws
Copy link
Contributor

@webwarrior-ws webwarrior-ws commented Jan 9, 2024

Add DisallowShadowing rule and tests for it.

Fixes #112

@webwarrior-ws webwarrior-ws force-pushed the disallow-shadowing-squashed branch from d25dadd to f349def Compare January 9, 2024 09:40
@knocte
Copy link
Collaborator

knocte commented Jan 9, 2024

@webwarrior-ws please add a test for shadowing for variables that start with underscore: in this case we don't want the rule to flag them.

@webwarrior-ws
Copy link
Contributor Author

@webwarrior-ws please add a test for shadowing for variables that start with underscore: in this case we don't want the rule to flag them.

Added new commit with test and changes to the rule code.

@knocte knocte changed the title Add DisallowShadowing rule DRAFT: Add DisallowShadowing rule Jan 10, 2024
@knocte
Copy link
Collaborator

knocte commented Jan 7, 2026

@webwarrior-ws let's rebase this again sorry.

@knocte knocte force-pushed the disallow-shadowing-squashed branch from 1fb6cc1 to 29b6c28 Compare January 8, 2026 07:27
@knocte
Copy link
Collaborator

knocte commented Jan 8, 2026

@webwarrior-ws let's rebase this again sorry.

Nevermind, I just did it myself. However, I didn't manage to fix the build entirely so please continue adding commits here. Thanks

@knocte knocte force-pushed the disallow-shadowing-squashed branch from a156b0f to b7928b7 Compare January 8, 2026 08:31
webwarrior-ws and others added 4 commits January 8, 2026 16:34
Added DisallowShadowing rule (no implementation yet). Added
tests for it.
Implemented DisallowShadowing rule, making previously
added tests pass.

Fixes fsprojects#112
In DisallowShadowing rule, ignore variables that start with
underscore (`_`). Added test for this case.
Co-authored-by: webwarrior-ws <reg@webwarrior.ws>
@knocte knocte force-pushed the disallow-shadowing-squashed branch from ac41076 to a52ab0c Compare January 8, 2026 08:40
@knocte
Copy link
Collaborator

knocte commented Jan 8, 2026

Not sure the rule is working well, it seems to be flagging as a violation when two functions have parameters with the same name (look at my last WIP commit that I just pushed).

@webwarrior-ws
Copy link
Contributor Author

Not sure the rule is working well, it seems to be flagging as a violation when two functions have parameters with the same name (look at my last WIP commit that I just pushed).

I've noticed that too and added a test case for it.
Now working on fixing the rule.

That makes sure that there is no false positive when different
non-nested functions have the same parameter name.
Utilities.rangeContainsOtherRange only took line numbers into
account, changed it to compare both line and column numbers.
@webwarrior-ws webwarrior-ws force-pushed the disallow-shadowing-squashed branch from 3a608c6 to 823cbe1 Compare January 8, 2026 11:44
That makes sure that there is no false positive for the same
identifier in match clause used multiple times that doesn't
shadow another identifier.
@knocte knocte force-pushed the disallow-shadowing-squashed branch from aa5d2be to 2523523 Compare January 8, 2026 12:49
@knocte knocte force-pushed the disallow-shadowing-squashed branch from 2523523 to 89d1aa5 Compare January 8, 2026 12:51
That makes sure that there is no false positive for identifiers
used as arguments to active pattern.
In match expressions with active pattern.
@knocte knocte force-pushed the disallow-shadowing-squashed branch 2 times, most recently from a8be63b to 3b7b887 Compare January 8, 2026 15:31
@knocte knocte force-pushed the disallow-shadowing-squashed branch 3 times, most recently from e17c16f to 9226ca3 Compare January 9, 2026 04:19
@knocte knocte force-pushed the disallow-shadowing-squashed branch from 9226ca3 to 4a7b5ef Compare January 9, 2026 10:47
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.

Warning for shadowed declarations

2 participants