Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve sonar code smells #273

Merged
merged 10 commits into from
Dec 27, 2018
15 changes: 10 additions & 5 deletions core/src/main/java/io/parsingdata/metal/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.io.ByteArrayOutputStream;
import java.math.BigInteger;
import java.util.Locale;
import java.util.Optional;
import java.util.zip.DataFormatException;
import java.util.zip.Inflater;
Expand All @@ -33,13 +34,13 @@

public final class Util {

final private static char[] HEX_ARRAY = "0123456789ABCDEF".toCharArray(); // Private because array content is mutable.
private static final char[] HEX_ARRAY = "0123456789ABCDEF".toCharArray(); // Private because array content is mutable.

private Util() {}

public static <T>T checkNotNull(final T argument, final String name) {
if (argument == null) {
throw new IllegalArgumentException("Argument " + name + " may not be null.");
throw new IllegalArgumentException(format("Argument %s may not be null.", name));
}
return argument;
}
Expand All @@ -48,15 +49,15 @@ public static <T>T[] checkContainsNoNulls(final T[] arguments, final String name
checkNotNull(arguments, name);
for (final T argument : arguments) {
if (argument == null) {
throw new IllegalArgumentException("Value in array " + name + " may not be null.");
throw new IllegalArgumentException(format("Value in array %s may not be null.", name));
}
}
return arguments;
}

public static String checkNotEmpty(final String argument, final String name) {
if (checkNotNull(argument, name).isEmpty()) {
throw new IllegalArgumentException("Argument " + name + " may not be empty.");
throw new IllegalArgumentException(format("Argument %s may not be empty.", name));
}
return argument;
}
Expand All @@ -68,11 +69,15 @@ public static boolean notNullAndSameClass(final Object object, final Object othe

public static BigInteger checkNotNegative(final BigInteger argument, final String name) {
if (checkNotNull(argument, name).compareTo(ZERO) < 0) {
throw new IllegalArgumentException("Argument " + name + " may not be negative.");
throw new IllegalArgumentException(format("Argument %s may not be negative.", name));
}
return argument;
}

public static String format(final String format, final Object... args) {
return String.format(Locale.ENGLISH, format, args);
}

public static String bytesToHexString(final byte[] bytes) {
checkNotNull(bytes, "bytes");
char[] hexChars = new char[bytes.length * 2];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import static io.parsingdata.metal.Util.checkNotNegative;
import static io.parsingdata.metal.Util.checkNotNull;
import static io.parsingdata.metal.Util.format;

import java.io.IOException;
import java.io.UncheckedIOException;
Expand All @@ -37,7 +38,7 @@ public class ByteStreamSource extends Source {
@Override
protected byte[] getData(final BigInteger offset, final BigInteger length) {
if (!isAvailable(offset, length)) {
throw new IllegalStateException("Data to read is not available ([offset=" + offset + ";length=" + length + ";source=" + this + ").");
throw new IllegalStateException(format("Data to read is not available ([offset=%d;length=%d;source=%s).", offset, length, this));
}
try {
return input.read(offset, length.intValueExact());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static io.parsingdata.metal.Trampoline.intermediate;
import static io.parsingdata.metal.Util.checkNotNegative;
import static io.parsingdata.metal.Util.checkNotNull;
import static io.parsingdata.metal.Util.format;

import java.math.BigInteger;
import java.util.Objects;
Expand Down Expand Up @@ -73,7 +74,7 @@ private static Trampoline<BigInteger> calculateTotalSize(final ImmutableList<Val
@Override
protected byte[] getData(final BigInteger offset, final BigInteger length) {
if (!isAvailable(offset, length)) {
throw new IllegalStateException("Data to read is not available (offset=" + offset + ";length=" + length + ";source=" + this + ").");
throw new IllegalStateException(format("Data to read is not available (offset=%d;length=%d;source=%s).", offset, length, this));
}
return getData(values, ZERO, ZERO, offset, length, new byte[length.intValueExact()]).computeResult();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static io.parsingdata.metal.Util.bytesToHexString;
import static io.parsingdata.metal.Util.checkNotNegative;
import static io.parsingdata.metal.Util.checkNotNull;
import static io.parsingdata.metal.Util.format;

import java.math.BigInteger;
import java.util.Arrays;
Expand All @@ -37,7 +38,7 @@ public ConstantSource(final byte[] data) {
@Override
protected byte[] getData(final BigInteger offset, final BigInteger length) {
if (!isAvailable(offset, length)) {
throw new IllegalStateException("Data to read is not available ([offset=" + offset + ";length=" + length + ";source=" + this + ").");
throw new IllegalStateException(format("Data to read is not available ([offset=%d;length=%d;source=%s).", offset, length, this));
}
final byte[] outputData = new byte[length.intValueExact()];
System.arraycopy(data, offset.intValueExact(), outputData, 0, outputData.length);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static io.parsingdata.metal.Trampoline.intermediate;
import static io.parsingdata.metal.Util.checkNotNegative;
import static io.parsingdata.metal.Util.checkNotNull;
import static io.parsingdata.metal.Util.format;

import java.math.BigInteger;
import java.util.Objects;
Expand Down Expand Up @@ -52,7 +53,7 @@ protected byte[] getData(final BigInteger offset, final BigInteger length) {
checkNotNegative(offset, "offset");
final byte[] data = getValue();
if (checkNotNegative(length, "length").add(offset).compareTo(BigInteger.valueOf(data.length)) > 0) {
throw new IllegalStateException("Data to read is not available ([offset=" + offset + ";length=" + length + ";source=" + this + ").");
throw new IllegalStateException(format("Data to read is not available ([offset=%d;length=%d;source=%s).", offset, length, this));
}
final byte[] outputData = new byte[length.intValueExact()];
System.arraycopy(data, offset.intValueExact(), outputData, 0, outputData.length);
Expand All @@ -68,12 +69,12 @@ private synchronized byte[] getValue() {
if (cache == null) {
final ImmutableList<Optional<Value>> results = dataExpression.eval(parseState, encoding);
if (results.size <= index) {
throw new IllegalStateException("ValueExpression dataExpression yields " + results.size + " result(s) (expected at least " + (index + 1) + ").");
throw new IllegalStateException(format("ValueExpression dataExpression yields %d result(s) (expected at least %d).", results.size, index+1));
}
cache = getValueAtIndex(results, index, 0)
.computeResult()
.map(Value::getValue)
.orElseThrow(() -> new IllegalStateException("ValueExpression dataExpression yields empty Value at index " + index + "."));
.orElseThrow(() -> new IllegalStateException(format("ValueExpression dataExpression yields empty Value at index %d.", index)));
}
return cache;
}
Expand Down
9 changes: 4 additions & 5 deletions core/src/main/java/io/parsingdata/metal/data/ParseState.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,14 @@

import static io.parsingdata.metal.Util.checkNotNegative;
import static io.parsingdata.metal.Util.checkNotNull;
import static io.parsingdata.metal.Util.format;
import static io.parsingdata.metal.data.Slice.createFromSource;

import java.math.BigInteger;
import java.util.Objects;
import java.util.Optional;

import io.parsingdata.metal.Util;
import io.parsingdata.metal.encoding.Encoding;
import io.parsingdata.metal.expression.value.ValueExpression;
import io.parsingdata.metal.token.Token;

public class ParseState {
Expand Down Expand Up @@ -60,7 +59,7 @@ public ParseState addBranch(final Token token) {

public ParseState closeBranch(final Token token) {
if (token.isIterable() && !iterations.head.left.equals(token)) {
throw new IllegalStateException(String.format("Cannot close branch for iterable token %s. Current iteration state is for token %s.", token.name, iterations.head.left.name));
throw new IllegalStateException(format("Cannot close branch for iterable token %s. Current iteration state is for token %s.", token.name, iterations.head.left.name));
}
return new ParseState(order.closeBranch(), source, offset, token.isIterable() ? iterations.tail : iterations);
}
Expand All @@ -81,8 +80,8 @@ public Optional<ParseState> seek(final BigInteger newOffset) {
return newOffset.compareTo(ZERO) >= 0 ? Optional.of(new ParseState(order, source, newOffset, iterations)) : Optional.empty();
}

public ParseState source(final ValueExpression dataExpression, final int index, final ParseState parseState, final Encoding encoding) {
return new ParseState(order, new DataExpressionSource(dataExpression, index, parseState, encoding), ZERO, iterations);
public ParseState withSource(final Source source) {
jvdb marked this conversation as resolved.
Show resolved Hide resolved
jvdb marked this conversation as resolved.
Show resolved Hide resolved
return new ParseState(order, source, ZERO, iterations);
}

public Optional<Slice> slice(final BigInteger length) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public BinaryValueExpression(final ValueExpression left, final ValueExpression r
this.right = checkNotNull(right, "right");
}

public abstract Optional<Value> eval(final Value left, final Value right, final ParseState parseState, final Encoding encoding);
public abstract Optional<Value> eval(final Value leftValue, final Value rightValue, final ParseState parseState, final Encoding encoding);

@Override
public ImmutableList<Optional<Value>> eval(final ParseState parseState, final Encoding encoding) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ public Cat(final ValueExpression left, final ValueExpression right) {
}

@Override
public Optional<Value> eval(final Value left, final Value right, final ParseState parseState, final Encoding encoding) {
return ConcatenatedValueSource.create(ImmutableList.create(Optional.of(left)).add(Optional.of(right)))
.flatMap(source -> createFromSource(source, ZERO, left.getLength().add(right.getLength())))
public Optional<Value> eval(final Value leftValue, final Value rightValue, final ParseState parseState, final Encoding encoding) {
return ConcatenatedValueSource.create(ImmutableList.create(Optional.of(leftValue)).add(Optional.of(rightValue)))
.flatMap(source -> createFromSource(source, ZERO, leftValue.getLength().add(rightValue.getLength())))
.map(source -> new Value(source, encoding));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,58 +33,58 @@
* A {@link ValueExpression} that expands a result by copying and concatenating
* it a specified amount of times.
* <p>
* An Expand expression has two operands: <code>base</code> and
* An Expand expression has two operands: <code>bases</code> and
* <code>count</code> (both {@link ValueExpression}s). Both operands are
* evaluated. An <code>IllegalStateException</code> is thrown if evaluating
* <code>count</code> yields more than a single value. Multiple copies of the
* result of evaluating <code>base</code> are concatenated. The amount of copies
* result of evaluating <code>bases</code> are concatenated. The amount of copies
* equals the result of evaluating <code>count</code>.
*/
public class Expand implements ValueExpression {

public final ValueExpression base;
public final ValueExpression bases;
public final ValueExpression count;

public Expand(final ValueExpression base, final ValueExpression count) {
this.base = checkNotNull(base, "base");
public Expand(final ValueExpression bases, final ValueExpression count) {
this.bases = checkNotNull(bases, "bases");
this.count = checkNotNull(count, "count");
}

@Override
public ImmutableList<Optional<Value>> eval(final ParseState parseState, final Encoding encoding) {
final ImmutableList<Optional<Value>> base = this.base.eval(parseState, encoding);
if (base.isEmpty()) {
return base;
final ImmutableList<Optional<Value>> baseList = bases.eval(parseState, encoding);
if (baseList.isEmpty()) {
return baseList;
}
final ImmutableList<Optional<Value>> count = this.count.eval(parseState, encoding);
if (count.size != 1 || !count.head.isPresent()) {
final ImmutableList<Optional<Value>> countList = count.eval(parseState, encoding);
if (countList.size != 1 || !countList.head.isPresent()) {
throw new IllegalArgumentException("Count must evaluate to a single non-empty value.");
}
return expand(base, count.head.get().asNumeric().intValueExact(), new ImmutableList<>()).computeResult();
return expand(baseList, countList.head.get().asNumeric().intValueExact(), new ImmutableList<>()).computeResult();
}

private Trampoline<ImmutableList<Optional<Value>>> expand(final ImmutableList<Optional<Value>> base, final int count, final ImmutableList<Optional<Value>> aggregate) {
if (count < 1) {
private Trampoline<ImmutableList<Optional<Value>>> expand(final ImmutableList<Optional<Value>> baseList, final int countValue, final ImmutableList<Optional<Value>> aggregate) {
if (countValue < 1) {
return complete(() -> aggregate);
}
return intermediate(() -> expand(base, count - 1, aggregate.add(base)));
return intermediate(() -> expand(baseList, countValue - 1, aggregate.add(baseList)));
}

@Override
public String toString() {
return getClass().getSimpleName() + "(" + base + "," + count + ")";
return getClass().getSimpleName() + "(" + bases + "," + count + ")";
}

@Override
public boolean equals(final Object obj) {
return Util.notNullAndSameClass(this, obj)
&& Objects.equals(base, ((Expand)obj).base)
&& Objects.equals(bases, ((Expand)obj).bases)
&& Objects.equals(count, ((Expand)obj).count);
}

@Override
public int hashCode() {
return Objects.hash(getClass(), base, count);
return Objects.hash(getClass(), bases, count);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,18 @@ public Fold(final ValueExpression values, final BinaryOperator<ValueExpression>

@Override
public ImmutableList<Optional<Value>> eval(final ParseState parseState, final Encoding encoding) {
final ImmutableList<Optional<Value>> initial = this.initial != null ? this.initial.eval(parseState, encoding) : new ImmutableList<>();
if (initial.size > 1) {
final ImmutableList<Optional<Value>> initialList = initial != null ? initial.eval(parseState, encoding) : new ImmutableList<>();
if (initialList.size > 1) {
return new ImmutableList<>();
}
final ImmutableList<Optional<Value>> values = prepareValues(this.values.eval(parseState, encoding));
if (values.isEmpty() || containsEmpty(values).computeResult()) {
return initial;
final ImmutableList<Optional<Value>> valueList = prepareValues(this.values.eval(parseState, encoding));
if (valueList.isEmpty() || containsEmpty(valueList).computeResult()) {
return initialList;
}
if (!initial.isEmpty()) {
return ImmutableList.create(fold(parseState, encoding, reducer, initial.head, values).computeResult());
if (!initialList.isEmpty()) {
return ImmutableList.create(fold(parseState, encoding, reducer, initialList.head, valueList).computeResult());
}
return ImmutableList.create(fold(parseState, encoding, reducer, values.head, values.tail).computeResult());
return ImmutableList.create(fold(parseState, encoding, reducer, valueList.head, valueList.tail).computeResult());
}

private Trampoline<Optional<Value>> fold(final ParseState parseState, final Encoding encoding, final BinaryOperator<ValueExpression> reducer, final Optional<Value> head, final ImmutableList<Optional<Value>> tail) {
Expand All @@ -92,7 +92,7 @@ private Trampoline<Boolean> containsEmpty(final ImmutableList<Optional<Value>> l
.orElseGet(() -> complete(() -> true));
}

protected abstract ImmutableList<Optional<Value>> prepareValues(ImmutableList<Optional<Value>> values);
protected abstract ImmutableList<Optional<Value>> prepareValues(ImmutableList<Optional<Value>> valueList);

protected abstract ValueExpression reduce(BinaryOperator<ValueExpression> reducer, Value head, Value tail);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ public FoldLeft(final ValueExpression values, final BinaryOperator<ValueExpressi
}

@Override
protected ImmutableList<Optional<Value>> prepareValues(final ImmutableList<Optional<Value>> values) {
return reverse(values);
protected ImmutableList<Optional<Value>> prepareValues(final ImmutableList<Optional<Value>> valueList) {
return reverse(valueList);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public FoldRight(final ValueExpression values, final BinaryOperator<ValueExpress
}

@Override
protected ImmutableList<Optional<Value>> prepareValues(final ImmutableList<Optional<Value>> values) {
return values;
protected ImmutableList<Optional<Value>> prepareValues(final ImmutableList<Optional<Value>> valueList) {
return valueList;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public Add(final ValueExpression left, final ValueExpression right) {
}

@Override
public Optional<Value> eval(final Value left, final Value right, final ParseState parseState, final Encoding encoding) {
return Optional.of(ConstantFactory.createFromNumeric(left.asNumeric().add(right.asNumeric()), encoding));
public Optional<Value> eval(final Value leftValue, final Value rightValue, final ParseState parseState, final Encoding encoding) {
return Optional.of(ConstantFactory.createFromNumeric(leftValue.asNumeric().add(rightValue.asNumeric()), encoding));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ public Div(final ValueExpression left, final ValueExpression right) {
}

@Override
public Optional<Value> eval(final Value left, final Value right, final ParseState parseState, final Encoding encoding) {
if (right.asNumeric().equals(ZERO)) {
public Optional<Value> eval(final Value leftValue, final Value rightValue, final ParseState parseState, final Encoding encoding) {
if (rightValue.asNumeric().equals(ZERO)) {
return Optional.empty();
}
return Optional.of(ConstantFactory.createFromNumeric(left.asNumeric().divide(right.asNumeric()), encoding));
return Optional.of(ConstantFactory.createFromNumeric(leftValue.asNumeric().divide(rightValue.asNumeric()), encoding));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ public Mod(final ValueExpression left, final ValueExpression right) {
}

@Override
public Optional<Value> eval(final Value left, final Value right, final ParseState parseState, final Encoding encoding) {
if (right.asNumeric().compareTo(ZERO) <= 0) {
public Optional<Value> eval(final Value leftValue, final Value rightValue, final ParseState parseState, final Encoding encoding) {
if (rightValue.asNumeric().compareTo(ZERO) <= 0) {
return Optional.empty();
}
return Optional.of(ConstantFactory.createFromNumeric(left.asNumeric().mod(right.asNumeric()), encoding));
return Optional.of(ConstantFactory.createFromNumeric(leftValue.asNumeric().mod(rightValue.asNumeric()), encoding));
}

}
Loading