Cics web command fix v2 #2469
Open
Cics web command fix v2 #2469
+959
−85
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mm-broadcom
force-pushed
the
CICS-WEB-Command-Fix-V2
branch
from
8083031
to
9185bf1
Compare
Copied the changes from the V1 branch to this one. (Too many commits and conflicts to rebase easily.)
Fixed an existing test as it was using a token/command CHARACTERSET which did not exist in the CICS Receive rule for Clients. Added some more flexibility to the new rules as well.
Initial outline of checks for WEB's commands. Committing to save changes so far.
Added a function to check for mutually exclusive rules.
More utility functions and overloads.
Added more WEB command rule verification, and fixes/consolidation of rules as well to correspond with the new checks.
mm-broadcom
force-pushed
the
CICS-WEB-Command-Fix-V2
branch
from
a94c878
to
79330b8
Compare
Allow cics_handle_response to be mixed in with each of the WEB command subtypes.
Only get the first word if prior rule seen. Avoids a lot of overhead in the case of a ParserRuleContext.
Due to how expansive and computationally expensive the .getText() call on a Parser Rule Context could be, it was removed. The list of options along with the context thereof _should_ provide enough information as to where the error is.
Change the multitude of if-else statements into a switch statement using the rule's index as the basis to go off of. This uses a primitive type rather than blindly asking every possible class if it is it.
Added complete duplicate checking for WEB commands.
Change to <Integer, ErrorSeverity> vs <String, ErrorSeverity>
Fixed checkHasRequired functions and overloads.
|
@slavek-kucera Would it be possible for you to review this? |
server/engine/src/main/antlr4/org/eclipse/lsp/cobol/implicitDialects/cics/CICSParser.g4
Outdated
Show resolved
Hide resolved
...in/java/org/eclipse/lsp/cobol/implicitDialects/cics/utility/CICSOptionsCheckBaseUtility.java
Outdated
Show resolved
Hide resolved
...in/java/org/eclipse/lsp/cobol/implicitDialects/cics/utility/CICSOptionsCheckBaseUtility.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| if (ParserRuleContext.class.isAssignableFrom(rule.getClass())) { | ||
| if (!((ParserRuleContext) rule).isEmpty()) { |
There was a problem hiding this comment.
I think this is also always true.
There was a problem hiding this comment.
Not always, we care if there is content not that the current element in the list/collection is of a given type.
There was a problem hiding this comment.
I don't think that the isEmpty does what you think.
https://github.com/antlr/antlr4/blob/933c4e0a0d4d5accfc2f9fa416e2c0558b4512e0/runtime/Java/src/org/antlr/v4/runtime/RuleContext.java#L100
There was a problem hiding this comment.
That is exactly why I call isEmpty as it checks to see if a given RuleContext or derivative thereof has been called within the given instance of the rule.
The purpose of this function is to take a collection of possible tokens and/or rules and see if more than one has been called. If so, insert an exception/warning message into the LSP at the given context to inform the user accordingly.
There was a problem hiding this comment.
- this expression will be always true
- neither of the top level conditions are true -
rulesis a generally "list of lists".
...in/java/org/eclipse/lsp/cobol/implicitDialects/cics/utility/CICSOptionsCheckBaseUtility.java
Outdated
Show resolved
Hide resolved
...in/java/org/eclipse/lsp/cobol/implicitDialects/cics/utility/CICSOptionsCheckBaseUtility.java
Outdated
Show resolved
Hide resolved
...in/java/org/eclipse/lsp/cobol/implicitDialects/cics/utility/CICSOptionsCheckBaseUtility.java
Outdated
Show resolved
Hide resolved
...ain/java/org/eclipse/lsp/cobol/implicitDialects/cics/utility/CICSWebOptionsCheckUtility.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| @Test | ||
| void testReadInvalid() { |
There was a problem hiding this comment.
Add a few more tests to covert the error cases.
server/engine/src/main/antlr4/org/eclipse/lsp/cobol/implicitDialects/cics/CICSParser.g4
Outdated
Show resolved
Hide resolved
Remove that function as checkPrerequisiteIsMet covers the same purpose with less overhead from the developer.
Add check for lists in checkMutuallyExclusiveOptions, only count them once per iteration. Set a boolean to denote if the current "rule" being checked is a list and if so, grab the first relevant entry to use for retrieving the locality.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implementation of IBM's grammar for the WEB CICS command such that fulfills all of those requirements but is flexible enough for what the compiler accepts.
Here is the V1 Branch for reference. (Due to the amount of time between creation, merge/updates from Development and such, it was easier to make a new branch and port those over rather than shoehorn it in.)
How Has This Been Tested?
Wrote tests with example calls based upon IBM's documentation.
Checklist: