gnodet-bot commented on code in PR #26362:
URL: https://github.com/apache/camel/pull/26362#discussion_r3999810119
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java:
##########
@@ -92,39 +92,393 @@ public boolean isCanonical() {
}
public List<Error> validate(File file) throws Exception {
- if (schema == null) {
- init();
- }
- try {
- var target = mapper.readTree(file);
- return validate(target);
- } catch (Exception e) {
- return List.of(parseError(e));
- }
+ // the same checks as for content, so the CLI and the tools report the
same
+ return validate(java.nio.file.Files.readString(file.toPath()));
}
public List<Error> validate(String content) throws Exception {
if (schema == null) {
init();
}
+ Error extra = extraDocument(content);
+ if (extra != null) {
+ return List.of(extra);
+ }
+ Error deps = jbangDirective(content);
+ if (deps != null) {
+ return List.of(deps);
+ }
+ if (content == null || content.isBlank() || content.lines().allMatch(l
-> l.isBlank() || l.trim().startsWith("#"))) {
+ return List.of(Error.builder().messageKey("empty").format(new
MessageFormat("{0}"))
+ .arguments("the file has no YAML: a Camel YAML file is a
list of entries, each starting with \"- \":"
+ + " - route:, - from:, - beans:, - rest:, -
onException:")
+ .build());
+ }
try {
var target = mapper.readTree(content);
return validate(target);
} catch (Exception e) {
- return List.of(parseError(e));
+ return List.of(parseError(e, content));
}
}
+ private static final java.util.regex.Pattern LINE_COLUMN
+ = java.util.regex.Pattern.compile("line:? (\\d+), column:?
(\\d+)");
+
+ /**
+ * A YAML parse error whose line is a line of text at column 1 after the
routes (an explanation appended to the
+ * file, or a markdown fence) says so; the parser's "while scanning a
simple key" does not.
+ */
+ static Error parseError(Exception e, String content) {
+ Error plain = parseError(e);
+ String msg = e.getMessage();
+ if (msg == null || content == null) {
+ return plain;
+ }
+ // the message names several positions (the collection being parsed,
then the token that broke it); the
+ // problem is at the last one
+ String[] lines = content.split("\n", -1);
+ int line = -1;
+ String text = null;
+ java.util.regex.Matcher m = LINE_COLUMN.matcher(msg);
+ while (m.find()) {
+ int l = Integer.parseInt(m.group(1));
+ if (!"1".equals(m.group(2)) || l < 2 || l > lines.length) {
+ continue;
+ }
+ String t = lines[l - 1].trim();
+ if (t.isEmpty() || t.startsWith("-") || t.startsWith("#") ||
t.startsWith("%")) {
+ continue;
+ }
+ line = l;
+ text = t;
+ }
+ if (text == null) {
+ // a value that continues after its closing quote: message: ">>> "
+ exchange.getIn().getBody()
+ java.util.regex.Matcher any = LINE_COLUMN.matcher(msg);
+ int last = -1;
+ while (any.find()) {
+ last = Integer.parseInt(any.group(1));
+ }
+ if (last >= 1 && last <= lines.length) {
+ String t = lines[last - 1];
+ java.util.regex.Matcher q
+ =
java.util.regex.Pattern.compile(":\\s*(\"(?:[^\"\\\\]|\\\\.)*\"|'[^']*')\\s*\\S").matcher(t);
+ if (q.find()) {
+ String key = t.trim().contains(":") ?
t.trim().substring(0, t.trim().indexOf(':')) : "the value";
+ return Error.builder()
+ .messageKey("parser")
+ .format(new MessageFormat("{0}"))
+ .arguments("line " + last + ": the value of " +
key + " continues after its closing quote"
+ + " (\"...\" + ...): a YAML value is
one string, there is no concatenation; a"
+ + " log message is a simple expression,
write it as one quoted text such as"
+ + " \">>> ${body}\"")
+ .build();
+ }
+ }
+ return plain;
+ }
+ String cleaned = msg.replace("\n", " ").replaceAll("\\s+", " ").trim();
+ int cut = cleaned.indexOf("in 'reader'");
+ String head = cut > 0 ? cleaned.substring(0, cut).trim() : cleaned;
+ return Error.builder()
+ .messageKey("parser")
+ .format(new MessageFormat("{0}"))
+ .arguments("line " + line + " is not YAML (\"" +
(text.length() > 40 ? text.substring(0, 40) + "..." : text)
+ + "\"): a route file holds only the YAML, put
explanations in a # comment or leave them out"
+ + " (" + head + ")")
+ .build();
+ }
+
+ /**
+ * {@code //DEPS org.apache.camel:camel-groovy} at the top of a YAML file:
JBang's Java directive, which YAML reads
+ * as a plain string so the whole file becomes one scalar ("string found,
array expected"). Name it, and say what a
+ * YAML file uses instead.
+ */
+ static Error jbangDirective(String content) {
+ if (content == null) {
+ return null;
+ }
+ String[] lines = content.split("\n", -1);
+ for (int i = 0; i < lines.length; i++) {
+ String t = lines[i].trim();
+ if (t.isEmpty() || t.startsWith("#")) {
+ continue;
+ }
+ if (t.startsWith("//DEPS") || t.startsWith("//JAVA") ||
t.startsWith("//SOURCES") || t.startsWith("//")) {
Review Comment:
💡 **Nit:** `t.startsWith("//")` subsumes the three preceding checks
(`//DEPS`, `//JAVA`, `//SOURCES`), making them dead code. If the intent is to
catch *any* `//` directive (reasonable — a first content line starting with
`//` is never valid YAML), the three specific prefixes can be dropped. If the
intent is to restrict to known JBang directives only, the trailing `||
t.startsWith("//")` should be removed.
The current behavior is correct either way (any `//` at the start of the
first content line is wrong in a Camel YAML file), but the explicit prefixes
suggest a narrower intent than what the code actually does.
##########
dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java:
##########
@@ -92,39 +92,393 @@ public boolean isCanonical() {
}
public List<Error> validate(File file) throws Exception {
- if (schema == null) {
- init();
- }
- try {
- var target = mapper.readTree(file);
- return validate(target);
- } catch (Exception e) {
- return List.of(parseError(e));
- }
+ // the same checks as for content, so the CLI and the tools report the
same
+ return validate(java.nio.file.Files.readString(file.toPath()));
}
public List<Error> validate(String content) throws Exception {
if (schema == null) {
init();
}
+ Error extra = extraDocument(content);
+ if (extra != null) {
+ return List.of(extra);
+ }
+ Error deps = jbangDirective(content);
+ if (deps != null) {
+ return List.of(deps);
+ }
+ if (content == null || content.isBlank() || content.lines().allMatch(l
-> l.isBlank() || l.trim().startsWith("#"))) {
+ return List.of(Error.builder().messageKey("empty").format(new
MessageFormat("{0}"))
+ .arguments("the file has no YAML: a Camel YAML file is a
list of entries, each starting with \"- \":"
+ + " - route:, - from:, - beans:, - rest:, -
onException:")
+ .build());
+ }
try {
var target = mapper.readTree(content);
return validate(target);
} catch (Exception e) {
- return List.of(parseError(e));
+ return List.of(parseError(e, content));
}
}
+ private static final java.util.regex.Pattern LINE_COLUMN
+ = java.util.regex.Pattern.compile("line:? (\\d+), column:?
(\\d+)");
+
+ /**
+ * A YAML parse error whose line is a line of text at column 1 after the
routes (an explanation appended to the
+ * file, or a markdown fence) says so; the parser's "while scanning a
simple key" does not.
+ */
+ static Error parseError(Exception e, String content) {
+ Error plain = parseError(e);
+ String msg = e.getMessage();
+ if (msg == null || content == null) {
+ return plain;
+ }
+ // the message names several positions (the collection being parsed,
then the token that broke it); the
+ // problem is at the last one
+ String[] lines = content.split("\n", -1);
+ int line = -1;
+ String text = null;
+ java.util.regex.Matcher m = LINE_COLUMN.matcher(msg);
+ while (m.find()) {
+ int l = Integer.parseInt(m.group(1));
+ if (!"1".equals(m.group(2)) || l < 2 || l > lines.length) {
+ continue;
+ }
+ String t = lines[l - 1].trim();
+ if (t.isEmpty() || t.startsWith("-") || t.startsWith("#") ||
t.startsWith("%")) {
+ continue;
+ }
+ line = l;
+ text = t;
+ }
+ if (text == null) {
+ // a value that continues after its closing quote: message: ">>> "
+ exchange.getIn().getBody()
+ java.util.regex.Matcher any = LINE_COLUMN.matcher(msg);
+ int last = -1;
+ while (any.find()) {
+ last = Integer.parseInt(any.group(1));
+ }
+ if (last >= 1 && last <= lines.length) {
+ String t = lines[last - 1];
+ java.util.regex.Matcher q
+ =
java.util.regex.Pattern.compile(":\\s*(\"(?:[^\"\\\\]|\\\\.)*\"|'[^']*')\\s*\\S").matcher(t);
+ if (q.find()) {
+ String key = t.trim().contains(":") ?
t.trim().substring(0, t.trim().indexOf(':')) : "the value";
+ return Error.builder()
+ .messageKey("parser")
+ .format(new MessageFormat("{0}"))
+ .arguments("line " + last + ": the value of " +
key + " continues after its closing quote"
+ + " (\"...\" + ...): a YAML value is
one string, there is no concatenation; a"
+ + " log message is a simple expression,
write it as one quoted text such as"
+ + " \">>> ${body}\"")
+ .build();
+ }
+ }
+ return plain;
+ }
+ String cleaned = msg.replace("\n", " ").replaceAll("\\s+", " ").trim();
+ int cut = cleaned.indexOf("in 'reader'");
+ String head = cut > 0 ? cleaned.substring(0, cut).trim() : cleaned;
+ return Error.builder()
+ .messageKey("parser")
+ .format(new MessageFormat("{0}"))
+ .arguments("line " + line + " is not YAML (\"" +
(text.length() > 40 ? text.substring(0, 40) + "..." : text)
+ + "\"): a route file holds only the YAML, put
explanations in a # comment or leave them out"
+ + " (" + head + ")")
+ .build();
+ }
+
+ /**
+ * {@code //DEPS org.apache.camel:camel-groovy} at the top of a YAML file:
JBang's Java directive, which YAML reads
+ * as a plain string so the whole file becomes one scalar ("string found,
array expected"). Name it, and say what a
+ * YAML file uses instead.
+ */
+ static Error jbangDirective(String content) {
+ if (content == null) {
+ return null;
+ }
+ String[] lines = content.split("\n", -1);
+ for (int i = 0; i < lines.length; i++) {
+ String t = lines[i].trim();
+ if (t.isEmpty() || t.startsWith("#")) {
+ continue;
+ }
+ if (t.startsWith("//DEPS") || t.startsWith("//JAVA") ||
t.startsWith("//SOURCES") || t.startsWith("//")) {
+ return Error.builder()
+ .messageKey("jbang")
+ .format(new MessageFormat("{0}"))
+ .arguments("line " + (i + 1) + ": " +
t.split("\\s+")[0] + " is read as text by YAML, so the whole"
+ + " file becomes one string; write it as a
YAML comment: # " + t
Review Comment:
💡 **Nit:** The error message always says "camel-jbang reads //DEPS inside
comments" and "add the dependency with --dep or camel.jbang.dependencies", even
when the matched directive is `//JAVA`, `//SOURCES`, or some other
`//`-prefixed line. For `//JAVA` or `//SOURCES`, the `--dep` advice doesn't
apply. Minor since the catch-all `//` already fires for all of these, but if
the specific prefixes are kept, the message could be tailored per directive
type.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
