[CSharp] Fix for #3510 -- accept grammars with either "->Channel(HIDDEN)" or "->channel(HIDDEN)" for CSharp #3547
Conversation
…uppercase "channel".
kaby76
changed the title
| INT : [0-9]+; | ||
| AAA : [a-z]+ -> channel(HIDDEN); // Note--lowercase "channel" | ||
| BBB : [A-Z]+ -> Channel(HIDDEN); // Note--uppercase "Channel" | ||
| WS : [ \t\r\n] -> skip; |
There was a problem hiding this comment.
It should be written using a descriptor (https://github.com/antlr/antlr4/tree/dev/runtime-testsuite/resources/org/antlr/v4/test/runtime/descriptors/LexerExec) to cover all runtimes but not C# only.
Also, please add tests on other commands if you can (see lexer-commands description):
- skip
- more
- popMode
- pushMode
- type
- channel
There was a problem hiding this comment.
OK, was wondering what those were for.
There was a problem hiding this comment.
BTW, we have similar tests but in tool-testsuite: https://github.com/antlr/antlr4/blob/dev/tool-testsuite/test/org/antlr/v4/test/tool/TestLexerActions.java But actually they should be moved to runtime-testsuite.
| @@ -0,0 +1,18 @@ | |||
| using Antlr4.Runtime; | |||
There was a problem hiding this comment.
It looks redundant (all files from runtime/CSharp/tests/issue-3510) since everything can be checked using a descriptor-based test.
There was a problem hiding this comment.
Agree. Will move this over to a descriptor.
What does this PR fix?
Discussion
The problem here revolves around an undocumented syntax in Antlr lexers. Use of "Channel(HIDDEN)" is accepted (in addition to the documented use of "channel(HIDDEN)") in a lexer grammar, but is outputted to the generated lexer as "_channel = HIDDEN". Unfortunately, this cannot work because the Antlr CSharp runtime sets the accessibility of the various fields ("_channel", "_type", "_mode") to private, and provides instead get and set properties. The generated code already uses the public method
PushMode(), and I don't want to change the visibility of the runtime for_channel, this change uses property set instead.The change is essentially to three lines in the .stg file for CSharp. I've added a test to the CSharp runtime that has a lexer use both "channel(HIDDEN)" and "Channel(HIDDEN)".
Why is this PR important?
In my opinion, this PR is not that important when compared to PRs to fix #3441, #3443, but I would include it in a long list of minor problems. There is no documentation describing "Channel()"; only "channel()" is documented. It occurs in grammars-v4/rego/RegoLexer.g4, which hasn't been targeted for CSharp only until recently.