wip: fix: change how modifiers are handled to use an ordered LinkedHashSet #4122
Conversation
|
@raghav-deepsource To avoid regressions of this behavior, could you add tests that verify that the insertion order of modifiers is preserved? Insertion order for all of |
|
|
||
| @Test | ||
| public void testAddModifier() { | ||
| inputModifierKinds.forEach(handler::addModifier); |
There was a problem hiding this comment.
I'm actually enjoying java for once thanks to this syntax!
There was a problem hiding this comment.
Why do you use class/instance fields and not local variables for modifiers, modifiersExt and so on?
There was a problem hiding this comment.
That was to reduce duplication. I had all of these tests within a single test function at one point which I refactored into its present form.
There was a problem hiding this comment.
though that has sort of come undone with current changes
|
|
||
| @Test | ||
| public void testAddModifier() { | ||
| inputModifierKinds.forEach(handler::addModifier); |
There was a problem hiding this comment.
Why do you use class/instance fields and not local variables for modifiers, modifiersExt and so on?
raghav-deepsource
force-pushed
the
modifier-fix
branch
from
25eb747 to
dc74c05
Compare
|
This PR was rather misguided, seeing as spoon generates modifiers as an unordered set anyway. A better method has been brought up in #4033. |
I should probably move the linkedHashSetCollector method elsewhere (a utility class for example).
this PR addresses #4033