Traverse duplicates - POC for automated duplicate checking

[chacebot]

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• TestCICSReceive.java
• TestCICSAllocate.java

Checklist:

• Each of my commits contains one meaningful change
• I have performed rebase of my branch on top of the development
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• I have checked my code and corrected any misspellings

[chacebot]

@slavek-kucera Happy to hear your thoughts on this. Checking tokens based on the arbitrary value of <900 is hackey but was looking for an avenue away from all the manual option checking

[slavek-kucera]

I assume that 897 is token id? This, in general, does not look like a good idea, the tokens could be reordered for number of reasons and this condition will change its meaning.

I think you'd need to construct a table of tokens you want to check using CICSLexer.tokenname.

[chacebot]

I did not create an efficient lookup table but did implement a fix to iterate over the ruleName arrays to match the token text

[slavek-kucera]

Is there a case when one of these keywords triggers warning in one rule, but error in another?
If not, we could have a single static map.
If so, we could still have a handful of static maps.

[chacebot]

This is all hit or miss depending on the command. I do not think it is safe to assume that the warning severities are uniform across the compiler for different commands with the same duplicate options. With that, i do not see the benefit of creating multiple static maps that mirror what we are passing as string args here.

[slavek-kucera]

The benefit would be exactly not creating the hashmap everytime the function is called.

[chacebot]

Actually good push for this, did not yet find a special case where duplicate warning keywords conflict behaviors across the commands i've seen so far. So, consolidated into a single static map.

[slavek-kucera]

The biggest problem with this approach is that we lose the ability to check the logical groups e.g. "INTO or SET".

Maybe it could be addressed by extending a map I talked about in the constext of checkDuplicateEntries.
It could map token_id to {group_id, severity} (by default group_id is the token text, severity is error).

The Map<Integer, TerminalNode> entries would be replaced just by Set<String> String being the group id e.g. "INTO or SET" or the token text.

[chacebot]

@slavek-kucera The logic for groups can still be handled manually with checkIllegalArguments. This might be too much of an effort to automate since we do not have a set standard on creating subgroups and how they are meant to represent the compiler's behavior.

[chacebot]

This could be an argument to ditch subgroups all together.

more changes