From 63707f9bd682c936912c6fbce45102337a51ec3f Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Mon, 2 Jan 2017 16:49:02 +0100 Subject: [PATCH] Fix Quality flaws Signed-off-by: Simon Brandhof --- .../java/org/sonar/search/EsSettings.java | 2 +- .../api/measurecomputer/MeasureImpl.java | 12 +++--- .../api/posttask/ConditionImpl.java | 2 +- .../component/MapBasedDbIdsRepository.java | 9 ++--- .../measure/MeasureVariations.java | 7 ++-- .../period/PeriodsHolderImpl.java | 7 ++-- .../task/projectanalysis/scm/DbScmInfo.java | 37 ++++++++----------- .../source/SourceLinesRepositoryImpl.java | 4 +- .../step/LoadMeasureComputersStep.java | 14 +++---- .../coverage/internal/DefaultCoverage.java | 12 +++--- .../api/server/rule/RulesDefinition.java | 5 ++- 11 files changed, 55 insertions(+), 56 deletions(-) diff --git a/server/sonar-search/src/main/java/org/sonar/search/EsSettings.java b/server/sonar-search/src/main/java/org/sonar/search/EsSettings.java index 5736ecd771f..03c430b1e07 100644 --- a/server/sonar-search/src/main/java/org/sonar/search/EsSettings.java +++ b/server/sonar-search/src/main/java/org/sonar/search/EsSettings.java @@ -183,7 +183,7 @@ public class EsSettings implements EsSettingsMBean { // If we're collecting indexing data send them to the Marvel host(s) if (!marvels.isEmpty()) { String hosts = StringUtils.join(marvels, ","); - LOGGER.info(String.format("Elasticsearch Marvel is enabled for %s", hosts)); + LOGGER.info("Elasticsearch Marvel is enabled for %s", hosts); builder.put("marvel.agent.exporter.es.hosts", hosts); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/api/measurecomputer/MeasureImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/api/measurecomputer/MeasureImpl.java index f4be3925088..27cc2b08678 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/api/measurecomputer/MeasureImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/api/measurecomputer/MeasureImpl.java @@ -25,6 +25,7 @@ import javax.annotation.concurrent.Immutable; import org.sonar.api.ce.measure.Measure; import static com.google.common.base.Preconditions.checkState; +import static java.lang.String.format; import static java.util.Objects.requireNonNull; import static org.sonar.server.computation.task.projectanalysis.measure.Measure.ValueType.BOOLEAN; import static org.sonar.server.computation.task.projectanalysis.measure.Measure.ValueType.DOUBLE; @@ -41,7 +42,7 @@ public class MeasureImpl implements Measure { public MeasureImpl(org.sonar.server.computation.task.projectanalysis.measure.Measure measure) { this.measure = requireNonNull(measure, "Measure couldn't be null"); - checkState(ALLOWED_VALUE_TYPES.contains(measure.getValueType()), String.format("Only following types are allowed %s", ALLOWED_VALUE_TYPES)); + checkState(ALLOWED_VALUE_TYPES.contains(measure.getValueType()), "Only following types are allowed %s", ALLOWED_VALUE_TYPES); } @Override @@ -75,10 +76,11 @@ public class MeasureImpl implements Measure { } private void checkValueType(org.sonar.server.computation.task.projectanalysis.measure.Measure.ValueType expected) { - checkState(measure.getValueType() == expected, String.format( - "Value can not be converted to %s because current value type is a %s", - expected.toString().toLowerCase(Locale.US), - measure.getValueType())); + if (measure.getValueType() != expected) { + throw new IllegalStateException(format("Value can not be converted to %s because current value type is a %s", + expected.toString().toLowerCase(Locale.US), + measure.getValueType())); + } } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/api/posttask/ConditionImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/api/posttask/ConditionImpl.java index 78ef18aebf8..3454df3e6b8 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/api/posttask/ConditionImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/api/posttask/ConditionImpl.java @@ -163,7 +163,7 @@ class ConditionImpl implements QualityGate.Condition { @Override public String getValue() { - checkState(status != NO_VALUE, "There is no value when status is " + NO_VALUE); + checkState(status != NO_VALUE, "There is no value when status is %s", NO_VALUE); return value; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/MapBasedDbIdsRepository.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/MapBasedDbIdsRepository.java index e19905947d8..1ef23c33832 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/MapBasedDbIdsRepository.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/MapBasedDbIdsRepository.java @@ -24,7 +24,6 @@ import java.util.HashMap; import java.util.Map; import static com.google.common.base.Preconditions.checkState; -import static java.lang.String.format; /** * Cache of database ids for components (component id and snapshot id) based in Maps. @@ -47,7 +46,7 @@ public final class MapBasedDbIdsRepository implements MutableDbIdsRepository T ref = componentToKey.apply(component); Long existingComponentId = componentIdsByRef.get(ref); checkState(existingComponentId == null, - format("Component id '%s' is already registered in repository for Component '%s', can not set new id '%s'", existingComponentId, component.getKey(), componentId)); + "Component id '%s' is already registered in repository for Component '%s', can not set new id '%s'", existingComponentId, component.getKey(), componentId); componentIdsByRef.put(ref, componentId); return this; } @@ -56,14 +55,14 @@ public final class MapBasedDbIdsRepository implements MutableDbIdsRepository public long getComponentId(Component component) { T ref = componentToKey.apply(component); Long componentId = componentIdsByRef.get(ref); - checkState(componentId != null, format("No component id registered in repository for Component '%s'", component.getKey())); + checkState(componentId != null, "No component id registered in repository for Component '%s'", component.getKey()); return componentId; } @Override public DbIdsRepository setDeveloperId(Developer developer, long developerId) { Long existingId = developerIdsByKey.get(developer); - checkState(existingId == null, format("Id '%s' is already registered in repository for Developer '%s', can not set new id '%s'", existingId, developer, developerId)); + checkState(existingId == null, "Id '%s' is already registered in repository for Developer '%s', can not set new id '%s'", existingId, developer, developerId); developerIdsByKey.put(developer, developerId); return this; } @@ -71,7 +70,7 @@ public final class MapBasedDbIdsRepository implements MutableDbIdsRepository @Override public long getDeveloperId(Developer developer) { Long devId = developerIdsByKey.get(developer); - checkState(devId != null, format("No id registered in repository for Developer '%s'", developer)); + checkState(devId != null, "No id registered in repository for Developer '%s'", developer); return devId; } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/measure/MeasureVariations.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/measure/MeasureVariations.java index ec94293013f..da67bcfa31f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/measure/MeasureVariations.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/measure/MeasureVariations.java @@ -21,14 +21,13 @@ package org.sonar.server.computation.task.projectanalysis.measure; import com.google.common.base.MoreObjects; import java.util.Arrays; +import java.util.Objects; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import org.sonar.server.computation.task.projectanalysis.period.Period; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; -import static com.google.common.base.Predicates.notNull; -import static com.google.common.collect.FluentIterable.from; @Immutable public final class MeasureVariations { @@ -38,7 +37,7 @@ public final class MeasureVariations { public MeasureVariations(Double... variations) { checkArgument(variations.length <= 5, "There can not be more than 5 variations"); - checkArgument(!from(Arrays.asList(variations)).filter(notNull()).isEmpty(), "There must be at least one variation"); + checkArgument(Arrays.stream(variations).anyMatch(Objects::nonNull), "There must be at least one variation"); for (Double variation : variations) { checkArgument(variation == null || !Double.isNaN(variation), NAN_ERROR_MESSAGE); } @@ -58,7 +57,7 @@ public final class MeasureVariations { public Builder setVariation(Period period, double variation) { int arrayIndex = period.getIndex() - 1; - checkState(variations[arrayIndex] == null, String.format("Variation for Period %s has already been set", period.getIndex())); + checkState(variations[arrayIndex] == null, "Variation for Period %s has already been set", period.getIndex()); checkArgument(!Double.isNaN(variation), NAN_ERROR_MESSAGE); variations[arrayIndex] = variation; return this; diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/period/PeriodsHolderImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/period/PeriodsHolderImpl.java index c8f016c536a..0f401e800a1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/period/PeriodsHolderImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/period/PeriodsHolderImpl.java @@ -20,12 +20,13 @@ package org.sonar.server.computation.task.projectanalysis.period; import com.google.common.base.Predicate; -import com.google.common.base.Predicates; import com.google.common.collect.Iterables; import java.util.Arrays; import java.util.List; +import java.util.Objects; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import org.sonar.core.util.stream.Collectors; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; @@ -64,7 +65,7 @@ public class PeriodsHolderImpl implements PeriodsHolder { @Override public List getPeriods() { checkHolderIsInitialized(); - return from(Arrays.asList(periods)).filter(Predicates.notNull()).toList(); + return Arrays.stream(periods).filter(Objects::nonNull).collect(Collectors.toList()); } @Override @@ -78,7 +79,7 @@ public class PeriodsHolderImpl implements PeriodsHolder { @Override public Period getPeriod(int i) { - checkState(hasPeriod(i), "Holder has no Period for index " + i); + checkState(hasPeriod(i), "Holder has no Period for index %s", i); return this.periods[i - 1]; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/DbScmInfo.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/DbScmInfo.java index 35f26c2ac33..b1fdd42a104 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/DbScmInfo.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/DbScmInfo.java @@ -19,21 +19,21 @@ */ package org.sonar.server.computation.task.projectanalysis.scm; -import com.google.common.base.Function; import com.google.common.base.Optional; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.function.Function; +import java.util.stream.StreamSupport; import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; +import org.sonar.core.util.stream.Collectors; import org.sonar.db.protobuf.DbFileSources; import org.sonar.server.computation.task.projectanalysis.component.Component; import static com.google.common.base.Preconditions.checkState; -import static com.google.common.base.Predicates.notNull; -import static com.google.common.collect.FluentIterable.from; -import static java.lang.String.format; /** * ScmInfo implementation based on the lines stored in DB @@ -49,16 +49,16 @@ class DbScmInfo implements ScmInfo { static Optional create(Component component, Iterable lines) { LineToChangeset lineToChangeset = new LineToChangeset(); - List lineChangesets = from(lines) - .transform(lineToChangeset) - .filter(notNull()) - .toList(); + List lineChangesets = StreamSupport.stream(lines.spliterator(), false) + .map(lineToChangeset) + .filter(Objects::nonNull) + .collect(Collectors.toList()); if (lineChangesets.isEmpty()) { return Optional.absent(); } checkState(!lineToChangeset.isEncounteredLineWithoutScmInfo(), - format("Partial scm information stored in DB for component '%s'. Not all lines have SCM info. Can not proceed", component)); - return Optional.of(new DbScmInfo(new ScmInfoImpl(lineChangesets))); + "Partial scm information stored in DB for component '%s'. Not all lines have SCM info. Can not proceed", component); + return Optional.of(new DbScmInfo(new ScmInfoImpl(lineChangesets))); } @Override @@ -95,23 +95,18 @@ class DbScmInfo implements ScmInfo { public Changeset apply(@Nonnull DbFileSources.Line input) { if (input.hasScmRevision() && input.hasScmDate()) { String revision = input.getScmRevision(); - Changeset changeset = cache.get(revision); - if (changeset == null) { - changeset = builder - .setRevision(revision) - .setAuthor(input.hasScmAuthor() ? input.getScmAuthor() : null) - .setDate(input.getScmDate()) - .build(); - cache.put(revision, changeset); - } - return changeset; + return cache.computeIfAbsent(revision, k -> builder + .setRevision(revision) + .setAuthor(input.hasScmAuthor() ? input.getScmAuthor() : null) + .setDate(input.getScmDate()) + .build()); } this.encounteredLineWithoutScmInfo = true; return null; } - public boolean isEncounteredLineWithoutScmInfo() { + boolean isEncounteredLineWithoutScmInfo() { return encounteredLineWithoutScmInfo; } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesRepositoryImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesRepositoryImpl.java index 44a3413c4ca..564b619f4d5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesRepositoryImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/SourceLinesRepositoryImpl.java @@ -44,7 +44,7 @@ public class SourceLinesRepositoryImpl implements SourceLinesRepository { Optional> linesIteratorOptional = reportReader.readFileSource(file.getReportAttributes().getRef()); - checkState(linesIteratorOptional.isPresent(), String.format("File '%s' has no source code", file)); + checkState(linesIteratorOptional.isPresent(), "File '%s' has no source code", file); int numberOfLines = reportReader.readComponent(file.getReportAttributes().getRef()).getLines(); CloseableIterator lineIterator = linesIteratorOptional.get(); @@ -59,7 +59,7 @@ public class SourceLinesRepositoryImpl implements SourceLinesRepository { private final int numberOfLines; private int currentLine = 0; - public ComponentLinesCloseableIterator(Component file, CloseableIterator lineIterator, int numberOfLines) { + private ComponentLinesCloseableIterator(Component file, CloseableIterator lineIterator, int numberOfLines) { this.file = file; this.delegate = lineIterator; this.numberOfLines = numberOfLines; diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadMeasureComputersStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadMeasureComputersStep.java index 5c5b8b40d82..918e0dbc904 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadMeasureComputersStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadMeasureComputersStep.java @@ -36,9 +36,9 @@ import org.sonar.api.measures.CoreMetrics; import org.sonar.api.measures.Metric; import org.sonar.api.measures.Metrics; import org.sonar.api.utils.dag.DirectAcyclicGraph; -import org.sonar.server.computation.task.projectanalysis.measure.MutableMeasureComputersHolder; import org.sonar.server.computation.task.projectanalysis.api.measurecomputer.MeasureComputerDefinitionImpl; import org.sonar.server.computation.task.projectanalysis.api.measurecomputer.MeasureComputerWrapper; +import org.sonar.server.computation.task.projectanalysis.measure.MutableMeasureComputersHolder; import org.sonar.server.computation.task.step.ComputationStep; import static com.google.common.base.Preconditions.checkState; @@ -111,7 +111,7 @@ public class LoadMeasureComputersStep implements ComputationStep { } private static void feedComputersByMetric(List wrappers, Map computersByOutputMetric, - Map computersByInputMetric) { + Map computersByInputMetric) { for (MeasureComputerWrapper computer : wrappers) { for (String outputMetric : computer.getDefinition().getOutputMetrics()) { computersByOutputMetric.put(outputMetric, computer); @@ -205,7 +205,7 @@ public class LoadMeasureComputersStep implements ComputationStep { @Override public boolean apply(@Nonnull String metric) { checkState(pluginMetricKeys.contains(metric) || CORE_METRIC_KEYS.contains(metric), - String.format("Metric '%s' cannot be used as an input metric as it's not a core metric and no plugin declare this metric", metric)); + "Metric '%s' cannot be used as an input metric as it's not a core metric and no plugin declare this metric", metric); return true; } } @@ -222,8 +222,8 @@ public class LoadMeasureComputersStep implements ComputationStep { private class ValidateOutputMetric implements Predicate { @Override public boolean apply(@Nonnull String metric) { - checkState(!CORE_METRIC_KEYS.contains(metric), String.format("Metric '%s' cannot be used as an output metric as it's a core metric", metric)); - checkState(pluginMetricKeys.contains(metric), String.format("Metric '%s' cannot be used as an output metric as no plugin declare this metric", metric)); + checkState(!CORE_METRIC_KEYS.contains(metric), "Metric '%s' cannot be used as an output metric as it's a core metric", metric); + checkState(pluginMetricKeys.contains(metric), "Metric '%s' cannot be used as an output metric as no plugin declare this metric", metric); return true; } } @@ -232,10 +232,10 @@ public class LoadMeasureComputersStep implements ComputationStep { private Set allOutputMetrics = new HashSet<>(); @Override - public boolean apply(@Nonnull MeasureComputerWrapper wrapper ) { + public boolean apply(@Nonnull MeasureComputerWrapper wrapper) { for (String outputMetric : wrapper.getDefinition().getOutputMetrics()) { checkState(!allOutputMetrics.contains(outputMetric), - String.format("Output metric '%s' is already defined by another measure computer '%s'", outputMetric, wrapper.getComputer())); + "Output metric '%s' is already defined by another measure computer '%s'", outputMetric, wrapper.getComputer()); allOutputMetrics.add(outputMetric); } return true; diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/coverage/internal/DefaultCoverage.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/coverage/internal/DefaultCoverage.java index bb0090fd8e8..fc84bd2fc5e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/coverage/internal/DefaultCoverage.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/coverage/internal/DefaultCoverage.java @@ -19,7 +19,6 @@ */ package org.sonar.api.batch.sensor.coverage.internal; -import com.google.common.base.Preconditions; import com.google.common.collect.Maps; import java.util.Collections; import java.util.SortedMap; @@ -31,6 +30,9 @@ import org.sonar.api.batch.sensor.coverage.NewCoverage; import org.sonar.api.batch.sensor.internal.DefaultStorable; import org.sonar.api.batch.sensor.internal.SensorStorage; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; + public class DefaultCoverage extends DefaultStorable implements NewCoverage { private DefaultInputFile inputFile; @@ -62,7 +64,7 @@ public class DefaultCoverage extends DefaultStorable implements NewCoverage { @Override public NewCoverage ofType(CoverageType type) { - Preconditions.checkNotNull(type, "type can't be null"); + checkNotNull(type, "type can't be null"); this.type = type; return this; } @@ -86,12 +88,12 @@ public class DefaultCoverage extends DefaultStorable implements NewCoverage { } private void validateLine(int line) { - Preconditions.checkState(line <= inputFile.lines(), String.format("Line %d is out of range in the file %s (lines: %d)", line, inputFile.relativePath(), inputFile.lines())); - Preconditions.checkState(line > 0, "Line number must be strictly positive: " + line); + checkState(line <= inputFile.lines(), "Line %s is out of range in the file %s (lines: %s)", line, inputFile.relativePath(), inputFile.lines()); + checkState(line > 0, "Line number must be strictly positive: %s", line); } private void validateFile() { - Preconditions.checkNotNull(inputFile, "Call onFile() first"); + checkNotNull(inputFile, "Call onFile() first"); } @Override diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java index 722b7ed6233..9a3fc8e4646 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/rule/RulesDefinition.java @@ -431,9 +431,10 @@ public interface RulesDefinition { private void registerRepository(NewRepositoryImpl newRepository) { Repository existing = repositoriesByKey.get(newRepository.key()); if (existing != null) { - checkState(existing.language().equals(newRepository.language), + String existingLanguage = existing.language(); + checkState(existingLanguage.equals(newRepository.language), "The rule repository '%s' must not be defined for two different languages: %s and %s", - newRepository.key, existing.language(), newRepository.language); + newRepository.key, existingLanguage, newRepository.language); } repositoriesByKey.put(newRepository.key, new RepositoryImpl(newRepository, existing)); } -- 2.39.5