Community
Participate
Working Groups
The following code is incorrect, because the precedence of the ternary operator (?:) is higher than assigment (=). However, the error message given is not for assignment or the ternary operator, but the less than or equal operator (<=): "The operator <= is undefined for the argument type(s) InputStream, FileInputStream" public abstract class BadWarning { InputStream inputStream; InputStream openInputStream() { return inputStream != null ? inputStream : inputStream = new FileInputStream(path); // Error here } } According to Java precedence, this should actually evaluate to return (inputStream != null ? inputStream : inputStream) = new FileInputStream(path); and the error should be "The left-hand side of an assignment must be a variable" -- so there seem to be two problems here: a wrong error being shown, and the correct error not being shown.
(Sorry, there is a missing "throws IOException" and a missing "String path" parameter to the function openInputStream -- but adding these does not change the error.) If you change the order of the ternary operator, no error is shown, as expected: return inputStream == null ? inputStream = new FileInputStream(path) : inputStream;
For the records, the compiler reports: : inputStream = new FileInputStream(path); // Error here ^ Syntax error on token "=", <= expected The error "The left-hand side of an assignment must be a variable" is a semantic error, that is detected during resolution, which in turn requires that the sources have been fully parsed. Due to the syntax error that is not possible. To verify the syntax error: after 'return' we need an expression. Comment 0 suggests to interpret the rest after 'return' as an AssignmentExpression, where the LHS is a ConditionalExpression. Here's the rule for AssignmentExpression: AssignmentExpression: ConditionalExpression Assignment Assignment: LeftHandSide AssignmentOperator Expression LeftHandSide: ExpressionName FieldAccess ArrayAccess We'd need to consume the conditional expression as a LeftHandSide, but there's no match. Now *parsing* goes left-to-right trying to find the largest legal chunk of text. Before we look at the '=' everything is legal, indeed, but after "'return' ConditionalExpression" there's no rule allowing a '=' token. Hence that's where we report the error. We could argue that the reported error is not "optimal", but to determine an "optimal" message we'd need to know what the author of the code *intended* to write, to tell her, what exactly must be changed. Obviously, knowing the author's intention is beyond the compiler. Prior to semantic analysis any binary operator like '<=' is indeed a proper repair to change the broken AssignmentExpression into a legal BinaryExpression. Long story short: - The expectation to see "The left-hand side of an assignment must be a variable" is wrong, because semantic analysis can't even start in the presence of a syntax error. - Alternative syntax errors are possible, but the parser cannot positively know which error is most helpful for the user. - The given error is correct. => Not a bug.
OK, not a bug, but rather than picking a random binary operator for repair, maybe you could pick an "undefined" binary operator, and that way, report a more useful error message in this case? The user should not need to know the exact grammar rules that the compiler is using, or even more so, the exact repair rules the compiler is using. I see your point as a compiler engineer, however from the user perspective, when there is no "<=" whatsoever in the code, this error *looks* plain wrong, whether or not it is as justifiable than any other error that could be given involving a randomly-chosen binary operator.
I agree "The left-hand side of an assignment must be a variable" cannot be expected. But I think the main problem is that the actual syntax error "Syntax error on token "=", <= expected" is only visible in the problems view - as hover you see the "The operator <= is undefined for the argument type(s) InputStream, FileInputStream" which is based on a symbol that error recovery invented. I wonder if we can somehow avoid creating the secondary error?
(In reply to Till Brychcy from comment #4) > I agree "The left-hand side of an assignment must be a variable" cannot be > expected. > > But I think the main problem is that the actual syntax error "Syntax error > on token "=", <= expected" is only visible in the problems view - as hover > you see the "The operator <= is undefined for the argument type(s) > InputStream, FileInputStream" which is based on a symbol that error recovery > invented. > > I wonder if we can somehow avoid creating the secondary error? Thanks for retesting. I only tested via batch compilation, but the difference between different compilation modes looks wrong.
I can reproduce the secondary error in a JUnit based on project.build(..) and project.findMarkers(..) The '<=' operator is indeed invented during syntax recovery. I don't see a way to influence this, nor is the replacement plain wrong. In fact it helps minimize secondary errors, as it establishes correct syntax, while we don't have sufficient information at that point how to create a replacement that's also semantically valid. The usability problem lies more in the UI: In hovers on left and right rulers we correctly see "Multiple markers at this line" including the syntax error. It is just quite difficult to see the syntax error in the editor, because its region is much smaller than the region of the secondary error. So, how can we make the syntax error more visible? - Suppress semantic errors in the very statement having the syntax error? (so far, only the MethodDeclaration is marked as having a syntax error, not the statement). - Improve selection of the most severe marker to show in the hover? No low hanging fruit for M2 and in fact not even a bug, just a request to improve usability.
I appreciate you taking a deeper look at this. Isn't it almost always appropriate to show syntax errors with precedence over overlapping semantic errors, since in this case, the semantic errors are probably a "domino effect" of the syntax errors?
(In reply to Luke Hutchison from comment #7) > I appreciate you taking a deeper look at this. Isn't it almost always > appropriate to show syntax errors with precedence over overlapping semantic > errors, since in this case, the semantic errors are probably a "domino > effect" of the syntax errors? Wrt "showing" in the editor: yes. My current thinking, however, is: continue reporting them (in Problems view and rulers), just change visualization in the editor, in case a syntax error and a semantic error meet on the same line. At this point it's less of a conceptual question, more a technical one: Where the information is shown in the editor we may not have the necessary information about the distinction syntactic / semantic error. And then it's a UI question: should we completely suppress the semantic error in the editor? Perhaps a general hint about "more problems at this location" might help - also for situations where two semantic errors overlap. OR, should we complete suppress reporting those "secondary" errors in the first place? In that case, "meet on the same line" may not be appropriate distinction, at this level we should more likely silence semantic errors for the enclosing statement. We have precedents like: don't report flow-related problems in the presence of resolve errors. etc.
(In reply to Stephan Herrmann from comment #8) > OR, should we complete suppress reporting those "secondary" errors in the > first place? In that case, "meet on the same line" may not be appropriate > distinction, at this level we should more likely silence semantic errors for > the enclosing statement. We have precedents like: don't report flow-related > problems in the presence of resolve errors. etc. We can do that if we have more such cases and it does seem like an easy way out. However, Would there ever be cases where the secondary error could be useful too?
Bulk move out of 4.9
The bug title says "syntax error" twice.. I think the first should say "semantic error"?
(In reply to Luke Hutchison from comment #11) > The bug title says "syntax error" twice.. I think the first should say > "semantic error"? No, I meant what I wrote :)
For a similar issue consider: //--- package bug; import java.util.Arrays; import java.util.List; public class Opstar { void m2(Object o1, Object o2) { List<Number> list = Arrays.asList(((Number) o1, (Number) o2)); } } //--- The compiler signals an error and when you hover over the error location, all you see is: "The operator * is undefined for the argument type(s) java.lang.Number, java.lang.Number" Since the source contains no '*' whatsoever, this leaves the user tearing their hair or biting their keyboard. Only when you hover the error icon in the ruler or look into the Problems view you see the primary error: "Syntax error on token ",", * expected" (which happens due to a bogus pair of enclosing parentheses). This example supports the idea that syntax errors should be shown in the editor with priority. Since the editor never displays hovers for several errors at the same location, I think it would be fine to simply not show any semantic errors that overlap/interact with a syntax error location, to at least ensure that all syntax errors are discoverable in the editor.
