gnodet-bot commented on code in PR #13554:
URL: https://github.com/apache/maven/pull/13554#discussion_r4296990622
##########
compat/maven-embedder/src/main/java/org/apache/maven/cli/props/MavenProperties.java:
##########
@@ -333,828 +66,35 @@ public boolean update(Map<String, String> props) {
}
public boolean update(MavenProperties properties) {
- boolean modified = false;
- // Remove "removed" properties from the cfg file
- for (String key : new ArrayList<String>(this.keySet())) {
- if (!properties.containsKey(key)) {
- this.remove(key);
- modified = true;
- }
- }
- // Update existing keys
- for (String key : properties.keySet()) {
- String v = this.get(key);
- List<String> comments = properties.getComments(key);
- List<String> value = properties.getRaw(key);
- if (v == null) {
- this.put(key, comments, value);
- modified = true;
- } else if (!v.equals(properties.get(key))) {
- if (comments.isEmpty()) {
- comments = this.getComments(key);
- }
- this.put(key, comments, value);
- modified = true;
- }
- }
- return modified;
- }
-
- public List<String> getRaw(String key) {
- if (layout.containsKey(key)) {
- if (layout.get(key).getValueLines() != null) {
- return new ArrayList<String>(layout.get(key).getValueLines());
- }
- }
- List<String> result = new ArrayList<String>();
- if (storage.containsKey(key)) {
- result.add(storage.get(key));
- }
- return result;
- }
-
- public List<String> getComments(String key) {
- if (layout.containsKey(key)) {
- if (layout.get(key).getCommentLines() != null) {
- return new
ArrayList<String>(layout.get(key).getCommentLines());
- }
- }
- return new ArrayList<String>();
+ return super.update(properties);
}
@Override
- public String remove(Object key) {
- Layout l = layout.get(key);
- if (l != null) {
- l.clearValue();
- }
- return storage.remove(key);
- }
-
- @Override
- public void clear() {
- for (Layout l : layout.values()) {
- l.clearValue();
- }
- storage.clear();
- }
-
- /**
- * Return the comment header.
- *
- * @return the comment header
- */
- public List<String> getHeader() {
- return header;
- }
-
- /**
- * Set the comment header.
- *
- * @param header the header to use
- */
- public void setHeader(List<String> header) {
- this.header = header;
- }
-
- /**
- * Return the comment footer.
- *
- * @return the comment footer
- */
- public List<String> getFooter() {
- return footer;
- }
-
- /**
- * Set the comment footer.
- *
- * @param footer the footer to use
- */
- public void setFooter(List<String> footer) {
- this.footer = footer;
- }
-
- /**
- * Reads a properties file and stores its internal structure. The found
- * properties will be added to the associated configuration object.
- *
- * @param in the reader to the properties file
- * @throws IOException if an error occurs
- */
- protected void loadLayout(Reader in, boolean maybeTyped) throws
IOException {
- PropertiesReader reader = new PropertiesReader(in, maybeTyped);
- boolean hasProperty = false;
- while (reader.nextProperty()) {
- hasProperty = true;
- storage.put(reader.getPropertyName(), reader.getPropertyValue());
- int idx = checkHeaderComment(reader.getCommentLines());
- layout.put(
- reader.getPropertyName(),
- new Layout(
- idx < reader.getCommentLines().size()
- ? new ArrayList<>(reader.getCommentLines()
- .subList(
- idx,
-
reader.getCommentLines().size()))
- : null,
- new ArrayList<>(reader.getValueLines())));
- }
- typed = maybeTyped && reader.typed != null && reader.typed;
- if (!typed) {
- for (Entry<String, String> e : storage.entrySet()) {
- e.setValue(unescapeJava(e.getValue()));
- }
- }
- if (hasProperty) {
- footer = new ArrayList<>(reader.getCommentLines());
- } else {
- header = new ArrayList<>(reader.getCommentLines());
- }
- if (substitute) {
- substitute();
- }
- }
-
- public void substitute() {
- substitute(callback);
- }
-
public void substitute(UnaryOperator<String> callback) {
- new DefaultInterpolator().interpolate(storage, callback);
- }
-
- /**
- * Writes the properties file to the given writer, preserving as much of
its
- * structure as possible.
- *
- * @param out the writer
- * @throws IOException if an error occurs
- */
- protected void saveLayout(Writer out, boolean typed) throws IOException {
- PropertiesWriter writer = new PropertiesWriter(out, typed);
- if (header != null) {
- for (String s : header) {
- writer.writeln(s);
- }
+ try {
+ super.substitute(callback);
+ } catch (StringInterpolatorException e) {
+ throw new InterpolatorException(e.getMessage());
Review Comment:
⚠️ **Cause chain dropped** — the `StringInterpolatorException` is discarded;
callers lose the stack trace.
```suggestion
throw new InterpolatorException(e.getMessage(), e);
```
##########
compat/maven-embedder/src/main/java/org/apache/maven/cli/props/MavenPropertiesLoader.java:
##########
@@ -19,150 +19,46 @@
package org.apache.maven.cli.props;
import java.io.IOException;
-import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.Enumeration;
import java.util.Map;
-import java.util.StringTokenizer;
import java.util.function.UnaryOperator;
-import org.apache.maven.impl.model.DefaultInterpolator;
+import org.apache.maven.api.services.InterpolatorException;
+import org.apache.maven.utils.StringInterpolatorException;
+/**
+ * @deprecated Use {@link org.apache.maven.utils.MavenPropertiesLoader}
instead.
+ */
@Deprecated
public class MavenPropertiesLoader {
+ public static final String INCLUDES_PROPERTY =
org.apache.maven.utils.MavenPropertiesLoader.INCLUDES_PROPERTY;
- public static final String INCLUDES_PROPERTY = "${includes}"; // includes
-
- public static final String OVERRIDE_PREFIX =
- "maven.override."; // prefix that marks that system property
should override defaults.
+ public static final String OVERRIDE_PREFIX =
org.apache.maven.utils.MavenPropertiesLoader.OVERRIDE_PREFIX;
public static void loadProperties(
java.util.Properties properties, Path path, UnaryOperator<String>
callback, boolean escape)
throws IOException {
- MavenProperties sp = new MavenProperties(false);
- if (Files.exists(path)) {
- sp.load(path);
+ try {
+
org.apache.maven.utils.MavenPropertiesLoader.loadProperties(properties, path,
callback, escape);
+ } catch (StringInterpolatorException e) {
+ throw new InterpolatorException(e.getMessage());
Review Comment:
⚠️ **Cause chain dropped** — same issue here and at lines 52 and 61 in this
file.
```suggestion
throw new InterpolatorException(e.getMessage(), e);
```
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/props/MavenPropertiesLoader.java:
##########
@@ -19,149 +19,46 @@
package org.apache.maven.cling.props;
import java.io.IOException;
-import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.Enumeration;
import java.util.Map;
-import java.util.StringTokenizer;
import java.util.function.UnaryOperator;
-import org.apache.maven.impl.model.DefaultInterpolator;
+import org.apache.maven.api.services.InterpolatorException;
+import org.apache.maven.utils.StringInterpolatorException;
+/**
+ * @deprecated Use {@link org.apache.maven.utils.MavenPropertiesLoader}
instead.
+ */
+@Deprecated
public class MavenPropertiesLoader {
+ public static final String INCLUDES_PROPERTY =
org.apache.maven.utils.MavenPropertiesLoader.INCLUDES_PROPERTY;
- public static final String INCLUDES_PROPERTY = "${includes}"; // includes
-
- public static final String OVERRIDE_PREFIX =
- "maven.override."; // prefix that marks that system property
should override defaults.
+ public static final String OVERRIDE_PREFIX =
org.apache.maven.utils.MavenPropertiesLoader.OVERRIDE_PREFIX;
public static void loadProperties(
java.util.Properties properties, Path path, UnaryOperator<String>
callback, boolean escape)
throws IOException {
- MavenProperties sp = new MavenProperties(false);
- if (Files.exists(path)) {
- sp.load(path);
+ try {
+
org.apache.maven.utils.MavenPropertiesLoader.loadProperties(properties, path,
callback, escape);
+ } catch (StringInterpolatorException e) {
+ throw new InterpolatorException(e.getMessage());
Review Comment:
⚠️ **Cause chain dropped** — same at lines 52 and 61 in this file.
```suggestion
throw new InterpolatorException(e.getMessage(), e);
```
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultInterpolator.java:
##########
@@ -201,236 +195,33 @@ public static String substVars(
UnaryOperator<String> callback,
BinaryOperator<String> postprocessor,
boolean defaultsToEmptyString) {
- return unescape(
- doSubstVars(val, currentKey, cycleMap, configProps, callback,
postprocessor, defaultsToEmptyString));
- }
-
- private static String doSubstVars(
- String val,
- String currentKey,
- Set<String> cycleMap,
- Map<String, String> configProps,
- UnaryOperator<String> callback,
- BinaryOperator<String> postprocessor,
- boolean defaultsToEmptyString) {
- if (val == null || val.isEmpty()) {
- return val;
- }
- if (cycleMap == null) {
- cycleMap = new HashSet<>();
- }
-
- // Put the current key in the cycle map.
- if (currentKey != null) {
- cycleMap.add(currentKey);
- }
-
- // Assume we have a value that is something like:
- // "leading ${foo.${bar}} middle ${baz} trailing"
-
- // Find the first ending '}' variable delimiter, which
- // will correspond to the first deepest nested variable
- // placeholder.
- int startDelim;
- int stopDelim = -1;
- do {
- stopDelim = val.indexOf(DELIM_STOP, stopDelim + 1);
- while (stopDelim > 0 && val.charAt(stopDelim - 1) == ESCAPE_CHAR) {
- stopDelim = val.indexOf(DELIM_STOP, stopDelim + 1);
- }
-
- // Find the matching starting "${" variable delimiter
- // by looping until we find a start delimiter that is
- // greater than the stop delimiter we have found.
- startDelim = val.indexOf(DELIM_START);
- while (stopDelim >= 0) {
- int idx = val.indexOf(DELIM_START, startDelim +
DELIM_START.length());
- if ((idx < 0) || (idx > stopDelim)) {
- break;
- } else if (idx < stopDelim) {
- startDelim = idx;
- }
- }
- } while (startDelim >= 0 && stopDelim >= 0 && stopDelim < startDelim +
DELIM_START.length());
-
- // If we do not have a start or stop delimiter, then just
- // return the existing value.
- if ((startDelim < 0) || (stopDelim < 0)) {
- cycleMap.remove(currentKey);
- return val;
- }
-
- // At this point, we have found a variable placeholder so
- // we must perform a variable substitution on it.
- // Using the start and stop delimiter indices, extract
- // the first, deepest nested variable placeholder.
- String variable = val.substring(startDelim + DELIM_START.length(),
stopDelim);
-
- String substValue =
- processSubstitution(variable, cycleMap, configProps, callback,
postprocessor, defaultsToEmptyString);
-
- // Append the leading characters, the substituted value of
- // the variable, and the trailing characters to get the new
- // value.
- val = val.substring(0, startDelim) + substValue +
val.substring(stopDelim + DELIM_STOP.length());
-
- // Now perform substitution again, since there could still
- // be substitutions to make.
- val = doSubstVars(val, currentKey, cycleMap, configProps, callback,
postprocessor, defaultsToEmptyString);
-
- cycleMap.remove(currentKey);
-
- // Return the value.
- return val;
- }
-
- private static String processSubstitution(
- String variable,
- Set<String> cycleMap,
- Map<String, String> configProps,
- UnaryOperator<String> callback,
- BinaryOperator<String> postprocessor,
- boolean defaultsToEmptyString) {
-
- // Process chained operators from left to right
- int startIdx = 0;
- String substValue = null;
-
- while (startIdx < variable.length()) {
- int idx1 = variable.indexOf(OP_DEFAULT, startIdx);
- int idx2 = variable.indexOf(OP_ALTERNATE, startIdx);
- int idx = idx1 >= 0 ? idx2 >= 0 ? Math.min(idx1, idx2) : idx1 :
idx2;
-
- if (idx < 0) {
- // No more operators, process the final variable
- if (substValue == null) {
- String currentVar = variable.substring(startIdx);
- substValue = resolveVariable(
- currentVar, cycleMap, configProps, callback,
postprocessor, defaultsToEmptyString);
- }
- break;
- }
-
- // Get the current variable part before the operator
- String varPart = variable.substring(startIdx, idx);
- if (substValue == null) {
- substValue =
- resolveVariable(varPart, cycleMap, configProps,
callback, postprocessor, defaultsToEmptyString);
- }
-
- // Find the end of the current operator's value
- int nextIdx1 = variable.indexOf(OP_DEFAULT, idx + 2);
- int nextIdx2 = variable.indexOf(OP_ALTERNATE, idx + 2);
- int nextIdx = nextIdx1 >= 0 ? nextIdx2 >= 0 ? Math.min(nextIdx1,
nextIdx2) : nextIdx1 : nextIdx2;
-
- String op = variable.substring(idx, idx + 2);
- String opValue = variable.substring(idx + 2, nextIdx >= 0 ?
nextIdx : variable.length());
-
- // Process the operator value through substitution if it contains
variables
- String processedOpValue = doSubstVars(
- opValue, variable, cycleMap, configProps, callback,
postprocessor, defaultsToEmptyString);
-
- // Apply the operator
- if (OP_ALTERNATE.equals(op)) {
- if (substValue != null && !substValue.isEmpty()) {
- substValue = processedOpValue;
- // Skip any remaining operators since we've made a decision
- break;
- }
- } else {
- // OP_DEFAULT: op can only be OP_DEFAULT here since idx was
found by indexOf
- if (substValue == null || substValue.isEmpty()) {
- substValue = processedOpValue;
- // Skip any remaining operators since we've made a decision
- break;
- }
- }
-
- startIdx = nextIdx >= 0 ? nextIdx : variable.length();
- }
-
- if (substValue == null) {
- if (defaultsToEmptyString) {
- substValue = "";
- } else {
- substValue = MARKER + "{" + variable + "}";
- }
+ try {
+ return StringInterpolator.substVars(
+ val, currentKey, cycleMap, configProps, callback,
postprocessor, defaultsToEmptyString);
+ } catch (StringInterpolatorException e) {
+ throw new InterpolatorException(e.getMessage());
Review Comment:
⚠️ **Cause chain dropped** — `InterpolatorException(String, Throwable)` is
available; pass `e` as the cause.
```suggestion
throw new InterpolatorException(e.getMessage(), e);
```
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/props/MavenProperties.java:
##########
@@ -333,828 +66,35 @@ public boolean update(Map<String, String> props) {
}
public boolean update(MavenProperties properties) {
- boolean modified = false;
- // Remove "removed" properties from the cfg file
- for (String key : new ArrayList<String>(this.keySet())) {
- if (!properties.containsKey(key)) {
- this.remove(key);
- modified = true;
- }
- }
- // Update existing keys
- for (String key : properties.keySet()) {
- String v = this.get(key);
- List<String> comments = properties.getComments(key);
- List<String> value = properties.getRaw(key);
- if (v == null) {
- this.put(key, comments, value);
- modified = true;
- } else if (!v.equals(properties.get(key))) {
- if (comments.isEmpty()) {
- comments = this.getComments(key);
- }
- this.put(key, comments, value);
- modified = true;
- }
- }
- return modified;
- }
-
- public List<String> getRaw(String key) {
- if (layout.containsKey(key)) {
- if (layout.get(key).getValueLines() != null) {
- return new ArrayList<String>(layout.get(key).getValueLines());
- }
- }
- List<String> result = new ArrayList<String>();
- if (storage.containsKey(key)) {
- result.add(storage.get(key));
- }
- return result;
- }
-
- public List<String> getComments(String key) {
- if (layout.containsKey(key)) {
- if (layout.get(key).getCommentLines() != null) {
- return new
ArrayList<String>(layout.get(key).getCommentLines());
- }
- }
- return new ArrayList<String>();
+ return super.update(properties);
}
@Override
- public String remove(Object key) {
- Layout l = layout.get(key);
- if (l != null) {
- l.clearValue();
- }
- return storage.remove(key);
- }
-
- @Override
- public void clear() {
- for (Layout l : layout.values()) {
- l.clearValue();
- }
- storage.clear();
- }
-
- /**
- * Return the comment header.
- *
- * @return the comment header
- */
- public List<String> getHeader() {
- return header;
- }
-
- /**
- * Set the comment header.
- *
- * @param header the header to use
- */
- public void setHeader(List<String> header) {
- this.header = header;
- }
-
- /**
- * Return the comment footer.
- *
- * @return the comment footer
- */
- public List<String> getFooter() {
- return footer;
- }
-
- /**
- * Set the comment footer.
- *
- * @param footer the footer to use
- */
- public void setFooter(List<String> footer) {
- this.footer = footer;
- }
-
- /**
- * Reads a properties file and stores its internal structure. The found
- * properties will be added to the associated configuration object.
- *
- * @param in the reader to the properties file
- * @throws IOException if an error occurs
- */
- protected void loadLayout(Reader in, boolean maybeTyped) throws
IOException {
- PropertiesReader reader = new PropertiesReader(in, maybeTyped);
- boolean hasProperty = false;
- while (reader.nextProperty()) {
- hasProperty = true;
- storage.put(reader.getPropertyName(), reader.getPropertyValue());
- int idx = checkHeaderComment(reader.getCommentLines());
- layout.put(
- reader.getPropertyName(),
- new Layout(
- idx < reader.getCommentLines().size()
- ? new ArrayList<>(reader.getCommentLines()
- .subList(
- idx,
-
reader.getCommentLines().size()))
- : null,
- new ArrayList<>(reader.getValueLines())));
- }
- typed = maybeTyped && reader.typed != null && reader.typed;
- if (!typed) {
- for (Entry<String, String> e : storage.entrySet()) {
- e.setValue(unescapeJava(e.getValue()));
- }
- }
- if (hasProperty) {
- footer = new ArrayList<>(reader.getCommentLines());
- } else {
- header = new ArrayList<>(reader.getCommentLines());
- }
- if (substitute) {
- substitute();
- }
- }
-
- public void substitute() {
- substitute(callback);
- }
-
public void substitute(UnaryOperator<String> callback) {
- new DefaultInterpolator().interpolate(storage, callback);
- }
-
- /**
- * Writes the properties file to the given writer, preserving as much of
its
- * structure as possible.
- *
- * @param out the writer
- * @throws IOException if an error occurs
- */
- protected void saveLayout(Writer out, boolean typed) throws IOException {
- PropertiesWriter writer = new PropertiesWriter(out, typed);
- if (header != null) {
- for (String s : header) {
- writer.writeln(s);
- }
+ try {
+ super.substitute(callback);
+ } catch (StringInterpolatorException e) {
+ throw new InterpolatorException(e.getMessage());
Review Comment:
⚠️ **Cause chain dropped** — mirror of the compat issue.
```suggestion
throw new InterpolatorException(e.getMessage(), e);
```
--
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]
