[
https://issues.apache.org/jira/browse/MDEP-931?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17954343#comment-17954343
]
ASF GitHub Bot commented on MDEP-931:
-------------------------------------
elharo commented on code in PR #530:
URL:
https://github.com/apache/maven-dependency-plugin/pull/530#discussion_r2109238647
##########
src/main/java/org/apache/maven/plugins/dependency/tree/DOTDependencyNodeVisitor.java:
##########
@@ -47,29 +48,38 @@ public DOTDependencyNodeVisitor(Writer writer) {
*/
@Override
public boolean visit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- writer.write("digraph \"" + node.toNodeString() + "\" { " +
System.lineSeparator());
- }
-
- // Generate "currentNode -> Child" lines
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ writer.write("digraph \"" + node.toNodeString() + "\" { " +
System.lineSeparator());
+ }
- List<DependencyNode> children = node.getChildren();
+ // Generate "currentNode -> Child" lines
- for (DependencyNode child : children) {
- writer.println("\t\"" + node.toNodeString() + "\" -> \"" +
child.toNodeString() + "\" ; ");
+ List<DependencyNode> children = node.getChildren();
+ for (DependencyNode child : children) {
+ writer.write("\t\"" + node.toNodeString() + "\" -> \"" +
child.toNodeString() + "\" ; "
+ + System.lineSeparator());
+ }
+ writer.flush();
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write to DOT output", e);
Review Comment:
[UncheckedIOException](https://docs.oracle.com/javase/8/docs/api/java/io/UncheckedIOException.html)
but really we need to figure out how to not thrown an exception at all here,
or perhaps change the method signature. Possibly we need to put everything in
a StringWriter first and then write the string later.
##########
src/main/java/org/apache/maven/plugins/dependency/tree/GraphmlDependencyNodeVisitor.java:
##########
@@ -63,37 +64,48 @@ public GraphmlDependencyNodeVisitor(Writer writer) {
* {@inheritDoc}
*/
@Override
- public boolean endVisit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- writer.write(GRAPHML_FOOTER);
- } else {
- DependencyNode p = node.getParent();
- writer.print("<edge source=\"" + generateId(p) + "\" target=\"" +
generateId(node) + "\">");
- if (node.getArtifact().getScope() != null) {
- // add Edge label
- writer.print("<data key=\"d1\"><y:PolyLineEdge><y:EdgeLabel>"
- + node.getArtifact().getScope() +
"</y:EdgeLabel></y:PolyLineEdge></data>");
+ public boolean visit(DependencyNode node) {
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ writer.write(GRAPHML_HEADER);
}
- writer.println("</edge>");
+ // write node
+ writer.write("<node id=\"" + generateId(node) + "\">");
+ // add node label
+ writer.write("<data key=\"d0\"><y:ShapeNode><y:NodeLabel>" +
node.toNodeString()
+ + "</y:NodeLabel></y:ShapeNode></data>");
+ writer.write("</node>" + System.lineSeparator());
+ writer.flush();
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write GraphML node", e);
Review Comment:
ditto
##########
src/main/java/org/apache/maven/plugins/dependency/tree/DOTDependencyNodeVisitor.java:
##########
@@ -47,29 +48,38 @@ public DOTDependencyNodeVisitor(Writer writer) {
*/
@Override
public boolean visit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- writer.write("digraph \"" + node.toNodeString() + "\" { " +
System.lineSeparator());
- }
-
- // Generate "currentNode -> Child" lines
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ writer.write("digraph \"" + node.toNodeString() + "\" { " +
System.lineSeparator());
+ }
- List<DependencyNode> children = node.getChildren();
+ // Generate "currentNode -> Child" lines
- for (DependencyNode child : children) {
- writer.println("\t\"" + node.toNodeString() + "\" -> \"" +
child.toNodeString() + "\" ; ");
+ List<DependencyNode> children = node.getChildren();
+ for (DependencyNode child : children) {
+ writer.write("\t\"" + node.toNodeString() + "\" -> \"" +
child.toNodeString() + "\" ; "
+ + System.lineSeparator());
Review Comment:
Output should be platform independent. That is, use \n
##########
src/main/java/org/apache/maven/plugins/dependency/tree/DOTDependencyNodeVisitor.java:
##########
@@ -47,29 +48,38 @@ public DOTDependencyNodeVisitor(Writer writer) {
*/
@Override
public boolean visit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- writer.write("digraph \"" + node.toNodeString() + "\" { " +
System.lineSeparator());
- }
-
- // Generate "currentNode -> Child" lines
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ writer.write("digraph \"" + node.toNodeString() + "\" { " +
System.lineSeparator());
+ }
- List<DependencyNode> children = node.getChildren();
+ // Generate "currentNode -> Child" lines
- for (DependencyNode child : children) {
- writer.println("\t\"" + node.toNodeString() + "\" -> \"" +
child.toNodeString() + "\" ; ");
+ List<DependencyNode> children = node.getChildren();
+ for (DependencyNode child : children) {
+ writer.write("\t\"" + node.toNodeString() + "\" -> \"" +
child.toNodeString() + "\" ; "
+ + System.lineSeparator());
+ }
+ writer.flush();
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write to DOT output", e);
}
-
- return true;
}
/**
* {@inheritDoc}
*/
@Override
public boolean endVisit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- writer.write(" } ");
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ writer.write(" } ");
+ writer.flush();
+ }
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write to DOT output", e);
Review Comment:
ditto
##########
src/main/java/org/apache/maven/plugins/dependency/tree/GraphmlDependencyNodeVisitor.java:
##########
@@ -63,37 +64,48 @@ public GraphmlDependencyNodeVisitor(Writer writer) {
* {@inheritDoc}
*/
@Override
- public boolean endVisit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- writer.write(GRAPHML_FOOTER);
- } else {
- DependencyNode p = node.getParent();
- writer.print("<edge source=\"" + generateId(p) + "\" target=\"" +
generateId(node) + "\">");
- if (node.getArtifact().getScope() != null) {
- // add Edge label
- writer.print("<data key=\"d1\"><y:PolyLineEdge><y:EdgeLabel>"
- + node.getArtifact().getScope() +
"</y:EdgeLabel></y:PolyLineEdge></data>");
+ public boolean visit(DependencyNode node) {
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ writer.write(GRAPHML_HEADER);
}
- writer.println("</edge>");
+ // write node
+ writer.write("<node id=\"" + generateId(node) + "\">");
+ // add node label
+ writer.write("<data key=\"d0\"><y:ShapeNode><y:NodeLabel>" +
node.toNodeString()
+ + "</y:NodeLabel></y:ShapeNode></data>");
+ writer.write("</node>" + System.lineSeparator());
+ writer.flush();
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write GraphML node", e);
}
- return true;
}
/**
* {@inheritDoc}
*/
@Override
- public boolean visit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- writer.write(GRAPHML_HEADER);
+ public boolean endVisit(DependencyNode node) {
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ writer.write(GRAPHML_FOOTER);
+ writer.flush();
+ } else {
+ DependencyNode p = node.getParent();
+ writer.write("<edge source=\"" + generateId(p) + "\"
target=\"" + generateId(node) + "\">");
+ if (node.getArtifact().getScope() != null) {
+ // add Edge label
+ writer.write("<data
key=\"d1\"><y:PolyLineEdge><y:EdgeLabel>"
+ + node.getArtifact().getScope() +
"</y:EdgeLabel></y:PolyLineEdge></data>");
+ }
+ writer.write("</edge>" + System.lineSeparator());
+ writer.flush();
+ }
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write GraphML edge or
footer", e);
Review Comment:
ditto
##########
src/main/java/org/apache/maven/plugins/dependency/tree/TGFDependencyNodeVisitor.java:
##########
@@ -101,31 +102,43 @@ public TGFDependencyNodeVisitor(Writer writer) {
* {@inheritDoc}
*/
@Override
- public boolean endVisit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- // dump edges on last node endVisit
- writer.println("#");
- for (EdgeAppender edge : edges) {
- writer.println(edge.toString());
- }
- } else {
- DependencyNode p = node.getParent();
- // using scope as edge label.
- edges.add(new EdgeAppender(p, node,
node.getArtifact().getScope()));
+ public boolean visit(DependencyNode node) {
+ try {
+ // write node
+ writer.write(generateId(node));
+ writer.write(" ");
+ writer.write(node.toNodeString());
+ writer.write(System.lineSeparator());
+ writer.flush();
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write TGF node", e);
}
- return true;
}
/**
* {@inheritDoc}
*/
@Override
- public boolean visit(DependencyNode node) {
- // write node
- writer.write(generateId(node));
- writer.write(" ");
- writer.println(node.toNodeString());
- return true;
+ public boolean endVisit(DependencyNode node) {
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ // dump edges on last node endVisit
+ writer.write("#" + System.lineSeparator());
Review Comment:
\n
##########
src/main/java/org/apache/maven/plugins/dependency/tree/TGFDependencyNodeVisitor.java:
##########
@@ -101,31 +102,43 @@ public TGFDependencyNodeVisitor(Writer writer) {
* {@inheritDoc}
*/
@Override
- public boolean endVisit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- // dump edges on last node endVisit
- writer.println("#");
- for (EdgeAppender edge : edges) {
- writer.println(edge.toString());
- }
- } else {
- DependencyNode p = node.getParent();
- // using scope as edge label.
- edges.add(new EdgeAppender(p, node,
node.getArtifact().getScope()));
+ public boolean visit(DependencyNode node) {
+ try {
+ // write node
+ writer.write(generateId(node));
+ writer.write(" ");
+ writer.write(node.toNodeString());
+ writer.write(System.lineSeparator());
+ writer.flush();
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write TGF node", e);
}
- return true;
}
/**
* {@inheritDoc}
*/
@Override
- public boolean visit(DependencyNode node) {
- // write node
- writer.write(generateId(node));
- writer.write(" ");
- writer.println(node.toNodeString());
- return true;
+ public boolean endVisit(DependencyNode node) {
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ // dump edges on last node endVisit
+ writer.write("#" + System.lineSeparator());
+ for (EdgeAppender edge : edges) {
+ writer.write(edge.toString());
+ writer.write(System.lineSeparator());
Review Comment:
\n
##########
src/main/java/org/apache/maven/plugins/dependency/tree/JsonDependencyNodeVisitor.java:
##########
@@ -56,13 +57,18 @@ public boolean visit(DependencyNode node) {
* @param node the node to write
*/
private void writeRootNode(DependencyNode node) {
- Set<DependencyNode> visited = new HashSet<>();
- int indent = 2;
- StringBuilder sb = new StringBuilder();
- sb.append("{").append("\n");
- writeNode(indent, node, sb, visited);
- sb.append("}").append("\n");
- writer.write(sb.toString());
+ try {
+ Set<DependencyNode> visited = new HashSet<>();
+ int indent = 2;
+ StringBuilder sb = new StringBuilder();
+ sb.append("{").append("\n");
+ writeNode(indent, node, sb, visited);
+ sb.append("}").append("\n");
+ writer.write(sb.toString());
+ writer.flush();
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write JSON output", e);
Review Comment:
ditto
##########
src/main/java/org/apache/maven/plugins/dependency/tree/TGFDependencyNodeVisitor.java:
##########
@@ -101,31 +102,43 @@ public TGFDependencyNodeVisitor(Writer writer) {
* {@inheritDoc}
*/
@Override
- public boolean endVisit(DependencyNode node) {
- if (node.getParent() == null || node.getParent() == node) {
- // dump edges on last node endVisit
- writer.println("#");
- for (EdgeAppender edge : edges) {
- writer.println(edge.toString());
- }
- } else {
- DependencyNode p = node.getParent();
- // using scope as edge label.
- edges.add(new EdgeAppender(p, node,
node.getArtifact().getScope()));
+ public boolean visit(DependencyNode node) {
+ try {
+ // write node
+ writer.write(generateId(node));
+ writer.write(" ");
+ writer.write(node.toNodeString());
+ writer.write(System.lineSeparator());
+ writer.flush();
+ return true;
+ } catch (IOException e) {
+ throw new RuntimeException("Failed to write TGF node", e);
}
- return true;
}
/**
* {@inheritDoc}
*/
@Override
- public boolean visit(DependencyNode node) {
- // write node
- writer.write(generateId(node));
- writer.write(" ");
- writer.println(node.toNodeString());
- return true;
+ public boolean endVisit(DependencyNode node) {
+ try {
+ if (node.getParent() == null || node.getParent() == node) {
+ // dump edges on last node endVisit
+ writer.write("#" + System.lineSeparator());
+ for (EdgeAppender edge : edges) {
+ writer.write(edge.toString());
+ writer.write(System.lineSeparator());
+ }
+ writer.flush();
+ } else {
+ DependencyNode p = node.getParent();
Review Comment:
p --> parent
> AbstractSerializingVisitor should not use PrintWriter
> -----------------------------------------------------
>
> Key: MDEP-931
> URL: https://issues.apache.org/jira/browse/MDEP-931
> Project: Maven Dependency Plugin (Moved to GitHub Issues)
> Issue Type: Improvement
> Reporter: Elliotte Rusty Harold
> Priority: Minor
>
> See https://github.com/apache/maven-dependency-plugin/pull/391/files
> AbstractSerializingVisitor uses a PrintWriter which swallows exceptions. It
> should use a regular Writer instead.
> This is exposed in the protected API so fixing this is ugly.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
