diff --git a/doc/changes/changes-4.0.0.md b/doc/changes/changes-4.0.0.md index c99e4048..46fe1cdf 100644 --- a/doc/changes/changes-4.0.0.md +++ b/doc/changes/changes-4.0.0.md @@ -11,6 +11,10 @@ * #76: Added `SELECT FROM VALUES ... AS` support. * #77: Added `SELECT FROM (SELECT ...)` support. +## Refactoring + +* #27: Fix sonar findings. + ## Dependency updates * Added `org.sonatype.ossindex.maven:ossindex-maven-plugin:3.1.0` diff --git a/src/main/java/com/exasol/datatype/type/IntervalDayToSecond.java b/src/main/java/com/exasol/datatype/type/IntervalDayToSecond.java index 9d23ce2c..7006b2b5 100644 --- a/src/main/java/com/exasol/datatype/type/IntervalDayToSecond.java +++ b/src/main/java/com/exasol/datatype/type/IntervalDayToSecond.java @@ -3,7 +3,7 @@ import com.exasol.sql.ColumnDefinitionVisitor; /** - * This class implements the Exasol-proprietary data type interval day to second + * This class implements the Exasol-proprietary data type interval day to second. */ public class IntervalDayToSecond implements DataType { private static final String NAME = "INTERVAL DAY(%s) TO SECOND(%s)"; @@ -48,7 +48,7 @@ public int getYearPrecision() { } /** - * @return millisecond precisiom + * @return millisecond precision */ public int getMillisecondPrecision() { return this.millisecondPrecision; diff --git a/src/main/java/com/exasol/datatype/value/IntervalDayToSecond.java b/src/main/java/com/exasol/datatype/value/IntervalDayToSecond.java index 90591409..af5d3dee 100644 --- a/src/main/java/com/exasol/datatype/value/IntervalDayToSecond.java +++ b/src/main/java/com/exasol/datatype/value/IntervalDayToSecond.java @@ -26,6 +26,7 @@ * This is also the recommended way to represent the interval values in other systems which do * not natively support this data type. */ +@java.lang.SuppressWarnings("squid:S4784") //regular expression is safe here public final class IntervalDayToSecond extends AbstractInterval { private static final int SIGN_MATCHING_GROUP = 1; private static final int DAYS_MATCHING_GROUP = 2; diff --git a/src/main/java/com/exasol/datatype/value/IntervalYearToMonth.java b/src/main/java/com/exasol/datatype/value/IntervalYearToMonth.java index a9bf737b..5ec6e9f3 100644 --- a/src/main/java/com/exasol/datatype/value/IntervalYearToMonth.java +++ b/src/main/java/com/exasol/datatype/value/IntervalYearToMonth.java @@ -23,6 +23,7 @@ * also the recommended way to represent the interval values in other systems which do not * natively support this data type. */ +@java.lang.SuppressWarnings("squid:S4784") //regular expression is safe here public final class IntervalYearToMonth extends AbstractInterval { private static final int SIGN_MATCHING_GROUP = 1; private static final int YEARS_MATCHING_GROUP = 2; diff --git a/src/main/java/com/exasol/sql/dql/select/rendering/SelectRenderer.java b/src/main/java/com/exasol/sql/dql/select/rendering/SelectRenderer.java index 173134ea..54ca7f68 100644 --- a/src/main/java/com/exasol/sql/dql/select/rendering/SelectRenderer.java +++ b/src/main/java/com/exasol/sql/dql/select/rendering/SelectRenderer.java @@ -105,20 +105,22 @@ public void visit(final OrderByClause orderByClause) { appendKeyWord(" ORDER BY "); appendListOfValueExpressions(orderByClause.getColumnReferences()); final Boolean desc = orderByClause.getDesc(); - appendStringDependingOnBoolean(desc, " DESC", " ASC"); + if (desc != null) { + appendStringDependingOnBoolean(desc, " DESC", " ASC"); + } final Boolean nullsFirst = orderByClause.getNullsFirst(); - appendStringDependingOnBoolean(nullsFirst, " NULLS FIRST", " NULLS LAST"); + if (nullsFirst != null) { + appendStringDependingOnBoolean(nullsFirst, " NULLS FIRST", " NULLS LAST"); + } setLastVisited(orderByClause); } - private void appendStringDependingOnBoolean(final Boolean booleanValue, final String string1, + private void appendStringDependingOnBoolean(final boolean booleanValue, final String string1, final String string2) { - if (booleanValue != null) { - if (booleanValue) { - appendKeyWord(string1); - } else { - appendKeyWord(string2); - } + if (booleanValue) { + appendKeyWord(string1); + } else { + appendKeyWord(string2); } } diff --git a/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java b/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java index f5f120d5..5b628599 100644 --- a/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java +++ b/src/main/java/com/exasol/sql/expression/rendering/AbstractExpressionRenderer.java @@ -61,11 +61,10 @@ protected void appendAutoQuoted(final String identifier) { } protected void appendCommaWhenNeeded(final ValueExpression valueExpression) { - if (this.lastVisited != null && !(valueExpression.getParent() instanceof BinaryArithmeticExpression)) { - if (this.lastVisited.isSibling(valueExpression) || (valueExpression.getParent() != this.lastVisited - && this.lastVisited.getClass().equals(valueExpression.getClass()))) { - append(", "); - } + if ((this.lastVisited != null && !(valueExpression.getParent() instanceof BinaryArithmeticExpression)) + && (this.lastVisited.isSibling(valueExpression) || (valueExpression.getParent() != this.lastVisited + && this.lastVisited.getClass().equals(valueExpression.getClass())))) { + append(", "); } } diff --git a/src/main/java/com/exasol/util/AbstractBottomUpTreeNode.java b/src/main/java/com/exasol/util/AbstractBottomUpTreeNode.java index e645a274..4806f966 100644 --- a/src/main/java/com/exasol/util/AbstractBottomUpTreeNode.java +++ b/src/main/java/com/exasol/util/AbstractBottomUpTreeNode.java @@ -1,21 +1,12 @@ package com.exasol.util; -import java.util.*; +import java.util.Arrays; +import java.util.List; /** * This is an abstract base class for nodes in a tree structure. */ -public abstract class AbstractBottomUpTreeNode implements TreeNode { - private TreeNode parent = null; - private final List children; - - /** - * Create a new instance of a {@link AbstractBottomUpTreeNode} that serves as leaf node for a tree. - */ - public AbstractBottomUpTreeNode() { - this.children = Collections.emptyList(); - } - +public abstract class AbstractBottomUpTreeNode extends AbstractTree { /** * Create a new instance of a {@link AbstractBottomUpTreeNode}. * @@ -68,49 +59,14 @@ public TreeNode getRoot() { } } - @Override - public TreeNode getParent() { - return this.parent; - } - @Override public void addChild(final TreeNode child) { throw new UnsupportedOperationException("Node \"" + child.toString() + "\" can only be added as child node in parent constructor in a bottom-up tree."); } - @Override - public List getChildren() { - return this.children; - } - - @Override - public TreeNode getChild(final int index) { - return this.children.get(index); - } - - @Override - public boolean isRoot() { - return (this == getRoot()); - } - - @Override - public boolean isChild() { - return (this.parent != null); - } - - @Override - public boolean isSibling(final TreeNode node) { - return (this.parent != null) && (this.getParent().getChildren().contains(node)); - } - @Override public void setParent(final TreeNode parent) { this.parent = parent; } - - @Override - public boolean isFirstSibling() { - return (this.parent != null) && (this.getParent().getChild(0) == this); - } } \ No newline at end of file diff --git a/src/main/java/com/exasol/util/AbstractTree.java b/src/main/java/com/exasol/util/AbstractTree.java new file mode 100644 index 00000000..6a5f41fa --- /dev/null +++ b/src/main/java/com/exasol/util/AbstractTree.java @@ -0,0 +1,47 @@ +package com.exasol.util; + +import java.util.ArrayList; +import java.util.List; + +/** + * An abstract base for {@link TreeNode} implementations. + */ +public abstract class AbstractTree implements TreeNode { + protected TreeNode parent = null; + protected List children = new ArrayList<>(); + + @Override + public TreeNode getParent() { + return this.parent; + } + + @Override + public List getChildren() { + return this.children; + } + + @Override + public TreeNode getChild(final int index) { + return this.children.get(index); + } + + @Override + public boolean isRoot() { + return (this == getRoot()); + } + + @Override + public boolean isChild() { + return (this.parent != null); + } + + @Override + public boolean isFirstSibling() { + return (this.parent != null) && (this.getParent().getChild(0) == this); + } + + @Override + public boolean isSibling(final TreeNode node) { + return (this.parent != null) && (this.getParent().getChildren().contains(node)); + } +} diff --git a/src/main/java/com/exasol/util/AbstractTreeNode.java b/src/main/java/com/exasol/util/AbstractTreeNode.java index cb3bd6e1..070b8106 100644 --- a/src/main/java/com/exasol/util/AbstractTreeNode.java +++ b/src/main/java/com/exasol/util/AbstractTreeNode.java @@ -1,15 +1,10 @@ package com.exasol.util; -import java.util.ArrayList; -import java.util.List; - /** * This is an abstract base class for nodes in a tree structure. */ -public abstract class AbstractTreeNode implements TreeNode { +public abstract class AbstractTreeNode extends AbstractTree { private TreeNode root; - private TreeNode parent; - private final List children = new ArrayList<>(); /** * Create a new instance of a {@link AbstractTreeNode} that serves as root for a tree. @@ -43,44 +38,9 @@ public TreeNode getRoot() { return this.root; } - @Override - public TreeNode getParent() { - return this.parent; - } - @Override public void addChild(final TreeNode child) { this.children.add(child); child.setParent(this); } - - @Override - public List getChildren() { - return this.children; - } - - @Override - public TreeNode getChild(final int index) { - return this.children.get(index); - } - - @Override - public boolean isRoot() { - return (this == getRoot()); - } - - @Override - public boolean isChild() { - return (this.parent != null); - } - - @Override - public boolean isFirstSibling() { - return (this.parent != null) && (this.getParent().getChild(0) == this); - } - - @Override - public boolean isSibling(final TreeNode node) { - return (this.parent != null) && (this.getParent().getChildren().contains(node)); - } } \ No newline at end of file diff --git a/src/test/java/com/exasol/sql/dml/merge/MatchedClauseTest.java b/src/test/java/com/exasol/sql/dml/merge/MatchedClauseTest.java index 0411100c..e26b8d78 100644 --- a/src/test/java/com/exasol/sql/dml/merge/MatchedClauseTest.java +++ b/src/test/java/com/exasol/sql/dml/merge/MatchedClauseTest.java @@ -7,7 +7,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class MatchedClauseTest { +class MatchedClauseTest { private MatchedClause matched; @BeforeEach diff --git a/src/test/java/com/exasol/sql/dml/merge/NotMatchedClauseTest.java b/src/test/java/com/exasol/sql/dml/merge/NotMatchedClauseTest.java index d0a5fb0b..c78020f3 100644 --- a/src/test/java/com/exasol/sql/dml/merge/NotMatchedClauseTest.java +++ b/src/test/java/com/exasol/sql/dml/merge/NotMatchedClauseTest.java @@ -7,7 +7,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class NotMatchedClauseTest { +class NotMatchedClauseTest { private NotMatchedClause notMatched; @BeforeEach diff --git a/src/test/java/com/exasol/sql/expression/TestBooleanTerm.java b/src/test/java/com/exasol/sql/expression/TestBooleanTerm.java index 5b903e60..58eec096 100644 --- a/src/test/java/com/exasol/sql/expression/TestBooleanTerm.java +++ b/src/test/java/com/exasol/sql/expression/TestBooleanTerm.java @@ -50,31 +50,39 @@ void testOperationFromUpperCaseYieldsNot() { // [utest->dsn~boolean-operators~1] @Test void testOperationFromUnknownOperatorThrowsException() { - assertThrows(IllegalArgumentException.class, () -> BooleanTerm.operation("illegal", not(true))); + final BooleanExpression notTrue = not(true); + assertThrows(IllegalArgumentException.class, () -> BooleanTerm.operation("illegal", notTrue)); } // [utest->dsn~boolean-operators~1] @Test void testOperationFromNotWithMoreOrLessThanOneOperandThrowsException() { - assertThrows(IllegalArgumentException.class, () -> BooleanTerm.operation("not", not(true), not(false))); + final BooleanExpression notTrue = not(true); + final BooleanExpression notFalse = not(false); + assertThrows(IllegalArgumentException.class, () -> BooleanTerm.operation("not", notTrue, notFalse)); } // [utest->dsn~boolean-operators~1] @Test void testOperationFromUpperCaseUnknownOperatorThrowsException() { - assertThrows(IllegalArgumentException.class, () -> BooleanTerm.operation("ILLEGAL", not(true))); + final BooleanExpression notTrue = not(true); + assertThrows(IllegalArgumentException.class, () -> BooleanTerm.operation("ILLEGAL", notTrue)); } // [utest->dsn~boolean-operators~1] @Test void testOperationFromUpperCaseNotWithMoreOrLessThanOneOperandThrowsException() { - assertThrows(IllegalArgumentException.class, () -> BooleanTerm.operation("NOT", not(true), not(false))); + final BooleanExpression notTrue = not(true); + final BooleanExpression notFalse = not(false); + assertThrows(IllegalArgumentException.class, () -> BooleanTerm.operation("NOT", notTrue, notFalse)); } // [utest->dsn~boolean-operators~1] @Test void testOperationFromNullOperatorThrowsException() { - assertThrows(NullPointerException.class, () -> BooleanTerm.operation(null, not(true), not(false))); + final BooleanExpression notTrue = not(true); + final BooleanExpression notFalse = not(false); + assertThrows(NullPointerException.class, () -> BooleanTerm.operation(null, notTrue, notFalse)); } // [utest->dsn~boolean-operation.comparison.constructing-from-strings~1] diff --git a/src/test/java/com/exasol/sql/expression/function/exasol/ExasolAggregateFunctionTest.java b/src/test/java/com/exasol/sql/expression/function/exasol/ExasolAggregateFunctionTest.java index 49590df7..fc478171 100644 --- a/src/test/java/com/exasol/sql/expression/function/exasol/ExasolAggregateFunctionTest.java +++ b/src/test/java/com/exasol/sql/expression/function/exasol/ExasolAggregateFunctionTest.java @@ -11,7 +11,7 @@ import com.exasol.sql.dql.select.Select; import com.exasol.sql.expression.BooleanTerm; -public class ExasolAggregateFunctionTest { +class ExasolAggregateFunctionTest { @Test void testAggregateFunctionCoalesce() { final Select select = StatementFactory.getInstance().select() // diff --git a/src/test/java/com/exasol/sql/expression/function/exasol/ExasolAnalyticFunctionTest.java b/src/test/java/com/exasol/sql/expression/function/exasol/ExasolAnalyticFunctionTest.java index 393b1ef3..390a6c55 100644 --- a/src/test/java/com/exasol/sql/expression/function/exasol/ExasolAnalyticFunctionTest.java +++ b/src/test/java/com/exasol/sql/expression/function/exasol/ExasolAnalyticFunctionTest.java @@ -11,7 +11,7 @@ import com.exasol.sql.dql.select.Select; import com.exasol.sql.expression.BooleanTerm; -public class ExasolAnalyticFunctionTest { +class ExasolAnalyticFunctionTest { @Test void testAggregateFunctionCoalesce() { final Select select = StatementFactory.getInstance().select() // diff --git a/src/test/java/com/exasol/util/TestAbstractBottomUpTreeNode.java b/src/test/java/com/exasol/util/TestAbstractBottomUpTreeNode.java index ff696465..6cceef77 100644 --- a/src/test/java/com/exasol/util/TestAbstractBottomUpTreeNode.java +++ b/src/test/java/com/exasol/util/TestAbstractBottomUpTreeNode.java @@ -63,8 +63,9 @@ void testIsFirstSiblingOnSecondChild() { } @Test - void testAddingChildAfterConstructurThrowsExpection() { - assertThrows(UnsupportedOperationException.class, () -> this.node.addChild(new DummyBottomUpTreeNode())); + void testAddingChildAfterConstructorThrowsException() { + final DummyBottomUpTreeNode child = new DummyBottomUpTreeNode(); + assertThrows(UnsupportedOperationException.class, () -> this.node.addChild(child)); } @Test diff --git a/src/test/java/com/exasol/util/TestAbstractTreeNode.java b/src/test/java/com/exasol/util/TestAbstractTreeNode.java index 31c20636..d2028c65 100644 --- a/src/test/java/com/exasol/util/TestAbstractTreeNode.java +++ b/src/test/java/com/exasol/util/TestAbstractTreeNode.java @@ -97,14 +97,13 @@ void testGetParent() { @Test void testSetParentToNullThrowsException() { - assertThrows(IllegalArgumentException.class, () -> new DummyTreeNode().setParent(null)); + final DummyTreeNode dummyTreeNode = new DummyTreeNode(); + assertThrows(IllegalArgumentException.class, () -> dummyTreeNode.setParent(null)); } @Test void testSetParentToSelfThrowsException() { - assertThrows(IllegalArgumentException.class, () -> { - final DummyTreeNode abstractNode = new DummyTreeNode(); - abstractNode.setParent(abstractNode); - }); + final DummyTreeNode abstractNode = new DummyTreeNode(); + assertThrows(IllegalArgumentException.class, () -> abstractNode.setParent(abstractNode)); } } \ No newline at end of file