Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(#3744): Human-Readable Error Message for Prohibited Comment in an Object Definition #3818

Merged
merged 14 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ void detectsCriticalErrorsSuccessfully(@Mktmp final Path temp) throws IOExceptio
new XMLDocument(
maven.result().get("target/2-shake/foo/x/main.xmir")
).nodes("//errors/error[@severity='critical']"),
Matchers.hasSize(1)
Matchers.hasSize(3)
);
}

Expand Down
2 changes: 1 addition & 1 deletion eo-parser/src/main/antlr4/org/eolang/parser/Eo.g4
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ metas
// Objects
// Ends on the next line
objects
: (object EOL?)* object
: (object EOL?)+
yegor256 marked this conversation as resolved.
Show resolved Hide resolved
;

comment
Expand Down
39 changes: 36 additions & 3 deletions eo-parser/src/main/java/org/eolang/parser/EoParserErrors.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,14 @@ private EoParserErrors(final List<ParsingException> errors, final Lines lines) {
this.lines = lines;
}

// @checkstyle ParameterNumberCheck (10 lines)
// @checkstyle ParameterNumberCheck (20 lines)
// @checkstyle CyclomaticComplexityCheck (100 lines)
// @todo #3744:30min Simplify {@link EoParserErrors#syntaxError} method.
// The method is too complex and has a high cognitive complexity. We need to simplify it.
// Don't forget to remove the @checkstyle CyclomaticComplexityCheck annotation and
// the @SuppressWarnings("PMD.CognitiveComplexity") annotation.
@Override
@SuppressWarnings("PMD.CognitiveComplexity")
public void syntaxError(
final Recognizer<?, ?> recognizer,
final Object symbol,
Expand All @@ -87,18 +93,22 @@ public void syntaxError(
);
}
final List<String> msgs = new ArrayList<>(0);
if (error instanceof NoViableAltException || error instanceof InputMismatchException) {
if (error instanceof NoViableAltException) {
final Token token = (Token) symbol;
final Parser parser = (Parser) recognizer;
final String rule = parser.getRuleInvocationStack().get(0);
final String[] names = parser.getRuleNames();
final String detailed;
if (names[EoParser.RULE_objects].equals(rule)) {
detailed = "Invalid object declaration";
detailed = "Invalid object list declaration";
} else if (names[EoParser.RULE_metas].equals(rule)) {
detailed = "Invalid meta declaration";
} else if (names[EoParser.RULE_program].equals(rule)) {
detailed = "Invalid program declaration";
} else if (names[EoParser.RULE_slave].equals(rule)) {
detailed = "Invalid objects declaration that may be used inside abstract object";
} else if (names[EoParser.RULE_object].equals(rule)) {
detailed = "Invalid object declaration";
} else {
detailed = "no viable alternative at input";
}
Expand All @@ -110,6 +120,29 @@ public void syntaxError(
Math.max(token.getStopIndex() - token.getStartIndex(), 1)
).formatted()
);
} else if (error instanceof InputMismatchException) {
final Token token = (Token) symbol;
final Parser parser = (Parser) recognizer;
final String rule = parser.getRuleInvocationStack().get(0);
final String detailed;
final String[] names = parser.getRuleNames();
if (names[EoParser.RULE_program].equals(rule)) {
detailed =
"We expected the program to end here but encountered something unexpected";
} else if (names[EoParser.RULE_objects].equals(rule)) {
detailed =
"We expected a list of objects here but encountered something unexpected";
} else {
detailed = msg;
}
msgs.add(new MsgLocated(line, position, detailed).formatted());
msgs.add(
new MsgUnderlined(
this.lines.line(line),
position,
Math.max(token.getStopIndex() - token.getStartIndex(), 1)
).formatted()
);
} else {
msgs.add(new MsgLocated(line, position, msg).formatted());
msgs.add(this.lines.line(line));
Expand Down
2 changes: 1 addition & 1 deletion eo-parser/src/main/java/org/eolang/parser/Lines.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,6 @@ String line(final int number) {
.map(UncheckedText::new)
.map(UncheckedText::asString);
}
return result.orElse("EOF");
return result.orElse("");
}
}
15 changes: 10 additions & 5 deletions eo-parser/src/main/java/org/eolang/parser/XeEoListener.java
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,16 @@ public void exitJustNamed(final EoParser.JustNamedContext ctx) {
}

@Override
@SuppressWarnings("PMD.ConfusingTernary")
public void enterAtom(final EoParser.AtomContext ctx) {
this.startObject(ctx)
.prop("atom", ctx.type().typeFqn().getText())
.leave();
final EoParser.TypeFqnContext fqn = ctx.type().typeFqn();
if (fqn == null) {
this.errors.add(XeEoListener.error(ctx, "Atom must have a type"));
this.startObject(ctx).leave();
} else {
this.startObject(ctx)
.prop("atom", fqn.getText())
.leave();
}
}

@Override
Expand Down Expand Up @@ -1256,7 +1261,7 @@ private static String trimMargin(final String text, final int indent) {
* Create parsing exception from given context.
* @param ctx Context
* @param msg Error message
* @return Parsing exception from current context
* @return Parsing exception from the current context
*/
private static ParsingException error(final ParserRuleContext ctx, final String msg) {
return new ParsingException(
Expand Down
12 changes: 6 additions & 6 deletions eo-parser/src/test/java/org/eolang/parser/EoSyntaxTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void printsProperListingEvenWhenSyntaxIsBroken() throws Exception {
"[] > x-н, 1\n"
);
MatcherAssert.assertThat(
EoIndentLexerTest.TO_ADD_MESSAGE,
"EO syntax is broken, but listing should be printed",
XhtmlMatchers.xhtml(
new String(
new EoSyntax(
Expand All @@ -100,7 +100,7 @@ void printsProperListingEvenWhenSyntaxIsBroken() throws Exception {
)
),
XhtmlMatchers.hasXPaths(
"/program/errors[count(error)=2]",
"/program/errors[count(error)=3]",
String.format("/program[listing='%s']", src)
)
);
Expand Down Expand Up @@ -279,10 +279,10 @@ void checksTypoPacks(final String yaml) {
if (story.map().containsKey(msg)) {
MatcherAssert.assertThat(
XhtmlMatchers.xhtml(story.after()).toString(),
story.after()
.xpath("/program/errors/error[1]/text()")
.get(0)
.replaceAll("\r", ""),
String.join(
"\n",
story.after().xpath("/program/errors/error/text()")
).replaceAll("\r", ""),
Matchers.equalTo(story.map().get(msg).toString())
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@
# SOFTWARE.
---
line: 2
message: >-
[2:4] error: 'Invalid object declaration'
message: |+
[2:4] error: 'Invalid objects declaration that may be used inside abstract object'
y:^
^
input: |
[3:-1] error: 'We expected the program to end here but encountered something unexpected'

input: |-
x
y:^
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
---
line: 2
message: |-
[2:7] error: 'Invalid object declaration'
[2:0] error: 'Atom must have a type'
[] > a [] > b [] > c [] > d hello world
^^^^^^^
[2:7] error: 'mismatched input '[' expecting '/''
[] > a [] > b [] > c [] > d hello world
^
input: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,17 @@
# SOFTWARE.
---
line: 5
# @todo #3706:30min The error message doesn't highlight the exact position of the
# comment. The error message should be updated to point to the exact position
# of the comment.
message: |
message: |-
[5:12] error: 'Invalid object declaration'
sprintwf

[5:12] error: 'Invalid objects declaration that may be used inside abstract object'
sprintwf

[4:-1] error: 'We expected the program to end here but encountered something unexpected'
# a comment here is prohibited
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

input: |
# No comments
[args] > app
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ line: 4
# Cryptic error message is returned when there are two empty lines between metas.
# The error message should be more informative and should highlight the exact location
# of the error.
message: |
[4:0] error: 'extraneous input '\n' expecting {COMMENTARY, 'Q', 'QQ', '*', '$', '[', '(', '@', '^', BYTES, STRING, INT, FLOAT, HEX, NAME, TEXT}'
message: |+
[4:0] error: 'We expected the program to end here but encountered something unexpected'

input: |
# No comments.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
---
line: 3
message: |-
[3:0] error: 'Invalid object declaration'
[3:0] error: 'We expected a list of objects here but encountered something unexpected'
+meta other
^^^^^^^^^^
input: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
---
line: 4
message: |-
[4:-1] error: 'Invalid program declaration'
[4:-1] error: 'We expected the program to end here but encountered something unexpected'
[] > inner
^^^^^^^^^^^^
input: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@
# SOFTWARE.
---
line: 3
message: |-
[3:-1] error: 'Invalid program declaration'
EOF
^^^
message: |+
[3:-1] error: 'We expected the program to end here but encountered something unexpected'

input: |
1.add 1 > x
(1.add 1) > y
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ line: 3
# The error message should be more informative and should point to the exact
# place in the input where the error occurred. Moreover it should be clear
# what to do to fix the error. 'simple-application-named.yaml' has the same issue.
message: |-
[3:-1] error: 'Invalid program declaration'
EOF
^^^
message: |+
[3:-1] error: 'We expected the program to end here but encountered something unexpected'

input: |
1.add 1 > x
(1.add 1)
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,22 @@
# SOFTWARE.
---
line: 5
# @todo #3744:60min Error Message Duplicates.
# As you can see, we have multiple error messages that are the
# same. We should remove duplicates and keep only meaningful error messages.
message: >-
[5:2] error: 'Invalid object declaration'
*
^
[5:2] error: 'Invalid object declaration'
*
^
[5:2] error: 'Invalid objects declaration that may be used inside abstract object'
*
^
[5:-1] error: 'We expected the program to end here but encountered something unexpected'
*
^^^^
input: |
# This is a code snippet from the following issue:
# https://github.com/objectionary/eo/issues/3332
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
---
line: 2
message: |-
[2:0] error: 'Invalid program declaration'
[2:0] error: 'We expected the program to end here but encountered something unexpected'
.z
^
input: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
---
line: 2
message: |-
[2:0] error: 'Invalid program declaration'
[2:0] error: 'We expected the program to end here but encountered something unexpected'
.z
^
input: |
Expand Down
Loading