ottlinger commented on code in PR #627:
URL: https://github.com/apache/creadur-rat/pull/627#discussion_r2927428349
##########
apache-rat-core/pom.xml:
##########
@@ -26,6 +26,9 @@
<packaging>jar</packaging>
<name>Apache Creadur RAT::Core</name>
<description>The core functionality of RAT that is used by all
clients.</description>
+ <properties>
+
<sonar.exclusions>src/main/java/org/apache/rat/report/xml/writer/XMLChar.java</sonar.exclusions>
Review Comment:
Would it make sense to add a comment here such as
"Code ist copied over from Xerces directly plus a reference to a RAT-xyz?"
Thanks, just for future people to give more context.
##########
apache-rat-tools/src/main/java/org/apache/rat/tools/xsd/XsdGenerator.java:
##########
@@ -71,16 +70,9 @@ public class XsdGenerator {
public static void main(final String[] args) throws IOException,
TransformerException {
XsdGenerator generator = new XsdGenerator();
- TransformerFactory tf = TransformerFactory.newInstance();
- Transformer transformer;
try (InputStream in = generator.getInputStream();
InputStream styleIn = StyleSheets.XML.getStyleSheet().get()) {
- transformer = tf.newTransformer(new StreamSource(styleIn));
- transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION,
"yes");
- transformer.setOutputProperty(OutputKeys.METHOD, "xml");
- transformer.setOutputProperty(OutputKeys.INDENT, "yes");
- transformer.setOutputProperty(OutputKeys.ENCODING, "UTF-8");
-
transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount";, "4");
+ Transformer transformer = StandardXmlFactory.create(styleIn);
Review Comment:
Thanks for addressing this duplicate code warning by putting it in a new
class.
##########
apache-rat-core/src/test/java/org/apache/rat/report/xml/writer/impl/base/XmlWriterTest.java:
##########
@@ -18,24 +18,28 @@
*/
package org.apache.rat.report.xml.writer.impl.base;
+import jdk.dynalink.Operation;
import org.apache.rat.report.xml.writer.InvalidXmlException;
import org.apache.rat.report.xml.writer.OperationNotAllowedException;
import org.apache.rat.report.xml.writer.XmlWriter;
+import org.apache.rat.testhelpers.XmlUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.w3c.dom.Document;
+import java.io.ByteArrayInputStream;
import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.NoSuchElementException;
import static org.assertj.core.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
-
public class XmlWriterTest {
Review Comment:
While scanning the changes in the writer package I wondered if there's
enough code coverage for the special cases handled in XmlWriter or if more
tests could be added (I only look at the code from the GitHub UI and not in an
IDE).
##########
apache-rat-core/src/main/java/org/apache/rat/report/xml/writer/XmlWriter.java:
##########
@@ -470,13 +126,10 @@ public IXmlWriter openElement(final CharSequence
elementName) throws IOException
@Override
public IXmlWriter comment(final CharSequence text) throws IOException {
- if (inElement) {
- writer.write('>');
- }
- inElement = false;
- writer.write("<!-- ");
- content(text);
- writer.write(" -->");
+ maybeCloseElement();
Review Comment:
Can you reformat this section or is it a UI problem in the web UI? (lines
seem to have different indentation)
##########
apache-rat-core/src/main/java/org/apache/rat/report/xml/writer/XmlWriter.java:
##########
@@ -675,49 +306,32 @@ public IXmlWriter closeDocument() throws IOException {
while (!elementNames.isEmpty()) {
closeElement();
}
- writer.flush();
return this;
}
- private void rawWrite(final CharSequence sequence) throws IOException {
- for (int i = 0; i < sequence.length(); i++) {
- final char charAt = sequence.charAt(i);
- writer.write(charAt);
+ @Override
+ public void close() throws IOException {
+ closeDocument();
+ if (appendable instanceof Closeable) {
Review Comment:
```
if (appendable instanceof Closeable) {
```
Sonarcloud's suggestion seems reasonable to me .... would you mind changing?
--
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]
