Java: `StringLiteral` matches String concatenation of two String literals

[Marcono1234]

It appears the class StringLiteral matches String concatenation of two String literals.
This is not how the JLS defines a String literal. Instead they should be matched as separate String literals being operands of an AddExpr (which is a CompileTimeConstantExpr).

Query finding these erroneous StringLiterals:

import java

from StringLiteral s
where
  exists(s.getLiteral().indexOf("+"))
  and not exists(s.getValue().indexOf("+"))
select s

Query Console link

[intrigus-lgtm]

This is mostly a problem with displaying the result, I guess.

If you have two string literals that are also CompileTimeConstantExpr's and you join both using + then java will join both at compile-time.
So "a" + "b" will be saved as "ab".
Have a look at this query:
https://lgtm.com/query/2434762471703982770/
You can see, that the first value of the result is "foo" + "bar" while the second value is "foobar".
So java joined the constants as expected, but lgtm shows it as if they were not joined.

[Marcono1234]

This is mostly a problem with displaying the result, I guess.

No, it is more than just displaying the result; it models the source code erroneously.
In my opinion CodeQL's primary goal should be to model the source code as close as possible, without introducing such 'shortcuts' as this one (in case that was the intention). If usage of these classes is too cumbersome, then introduce predicates or new classes to make usage easier (e.g. CompileTimeConstantExpr).

If you have two string literals that are also CompileTimeConstantExpr's and you join both using + then java will join both at compile-time.
So "a" + "b" will be saved as "ab".

Yes, but 1 + 2 will also have the constant value 3 in the .class file, yet it is correctly modeled as AddExpr.

[aschackmull]

This is intentional. That section of the JLS also has the following statement:

Strings concatenated from constant expressions (§15.29) are computed at compile time and then treated as if they were literals.

(For example, they are also interned). So I think it is quite reasonable to keep the behaviour as-is. Of course, the qldoc could be improved to clearly specify this choice.

Another reason is that the extractor builds on javac, which I believe has already performed the concatenation at the time when we traverse the parse-tree.

[Marcono1234]

Thanks for the JLS reference, though it appears this statement is (only?) in the context of String interning.
Further at the top it explicitly has this example:

"This is a " +        // actually a string-valued constant expression,
    "two-line string"    // formed from two string literals

Though if this is a limitation with javac, I guess it is fine to leave it as is, however a documentation change clarifying this would be good then indeed.

[aeisenberg]

Added the documentation label since this issue is about adding a missing bit of documentation.