PHP target generates invalid output when $ is used as part of the literal in lexer rule

[adamwojs]

Steps to reproduce

Generate lexer using PHP target from the following grammar

lexer grammar TestLexer;

DOLAR_ACTION: '$ACTION';

using ANTLR build from the current master branch

Actual behavior

ANTLR produces invalid PHP code (it's not allowed to use variables as part of the constant expression)

final class TestLexer extends Lexer
{
        # ...

	/**
	 * @var array<string|null>
	 */
	private const LITERAL_NAMES = [
	    null, "'$ACTION'"  
	];

        # ...
 }

PHP Fatal error:  Constant expression contains invalid operations in /home/awojs/Projekty/grammars-v4/tsql/target/TestLexer.php on line 48

Expected behavior

$ character in PHP Target is escaped

final class TestLexer extends Lexer
{
    # ...

	/**
	 * @var array<string|null>
	 */
	private const LITERAL_NAMES = [
	    null, "'\$ACTION'"
	];

    # ...
 }

Patch proposal

This seems to solve a problem

diff --git a/tool/src/org/antlr/v4/codegen/target/PHPTarget.java b/tool/src/org/antlr/v4/codegen/target/PHPTarget.java
index 2617565fb..6a9ba31d9 100644
--- a/tool/src/org/antlr/v4/codegen/target/PHPTarget.java
+++ b/tool/src/org/antlr/v4/codegen/target/PHPTarget.java
@@ -102,4 +102,11 @@ public class PHPTarget extends Target {
        protected void appendUnicodeEscapedCodePoint(int codePoint, StringBuilder sb) {
                UnicodeEscapes.appendPythonStyleEscapedCodePoint(codePoint, sb);
        }
+
+       @Override
+       public String getTargetStringLiteralFromANTLRStringLiteral(CodeGenerator generator, String literal, boolean addQuotes) {
+               String targetStringLiteral = super.getTargetStringLiteralFromANTLRStringLiteral(generator, literal, addQuotes);
+               targetStringLiteral = targetStringLiteral.replace("$", "\\$");
+               return targetStringLiteral;
+       }
 }

but maybe problem is more generic and getTargetStringLiteralFromANTLRStringLiteral should take into account all characters from targetCharValueEscape ?

[marcospassos]

Hey @adamwojs, sorry for the delayed reply.

I think escaping in getTargetStringLiteralFromANTLRStringLiteral should be enough. Could you please open a PR including tests?