[
https://issues.apache.org/jira/browse/BEAM-4307?focusedWorklogId=105939&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-105939
]
ASF GitHub Bot logged work on BEAM-4307:
----------------------------------------
Author: ASF GitHub Bot
Created on: 25/May/18 16:01
Start Date: 25/May/18 16:01
Worklog Time Spent: 10m
Work Description: lukecwik closed pull request #5483: [BEAM-4307] Enforce
ErrorProne analysis in runners-core-construction-java
URL: https://github.com/apache/beam/pull/5483
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/runners/core-construction-java/build.gradle
b/runners/core-construction-java/build.gradle
index 42f2f0a85f8..368f501ec9f 100644
--- a/runners/core-construction-java/build.gradle
+++ b/runners/core-construction-java/build.gradle
@@ -17,7 +17,7 @@
*/
apply from: project(":").file("build_rules.gradle")
-applyJavaNature()
+applyJavaNature(failOnWarning: true)
description = "Apache Beam :: Runners :: Core Construction Java"
ext.summary = """Beam Runners Core provides utilities to aid runner authors
interact with a Pipeline
@@ -48,6 +48,7 @@ dependencies {
shadow library.java.slf4j_api
shadow library.java.grpc_core
shadow library.java.grpc_stub
+ testCompileOnly library.java.findbugs_annotations
shadowTest library.java.hamcrest_core
shadowTest library.java.junit
shadowTest library.java.mockito_core
diff --git
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/CoderTranslation.java
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/CoderTranslation.java
index 4806fb416b1..3f858036049 100644
---
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/CoderTranslation.java
+++
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/CoderTranslation.java
@@ -27,7 +27,6 @@
import com.google.protobuf.ByteString;
import java.io.IOException;
import java.util.ArrayList;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.ServiceLoader;
@@ -135,7 +134,7 @@
private static Coder<?> fromKnownCoder(RunnerApi.Coder coder,
RehydratedComponents components)
throws IOException {
String coderUrn = coder.getSpec().getSpec().getUrn();
- List<Coder<?>> coderComponents = new LinkedList<>();
+ List<Coder<?>> coderComponents = new ArrayList<>();
for (String componentId : coder.getComponentCoderIdsList()) {
Coder<?> innerCoder = components.getCoder(componentId);
coderComponents.add(innerCoder);
diff --git
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/CoderTranslators.java
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/CoderTranslators.java
index 0b8f67ef296..ce3d11c5355 100644
---
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/CoderTranslators.java
+++
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/CoderTranslators.java
@@ -107,6 +107,7 @@ public T fromComponents(List<Coder<?>> components) {
public abstract static class SimpleStructuredCoderTranslator<T extends
Coder<?>>
implements CoderTranslator<T> {
+ @Override
public final T fromComponents(List<Coder<?>> components, byte[] payload) {
return fromComponents(components);
}
diff --git
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/PTransformTranslation.java
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/PTransformTranslation.java
index 37886605296..b1d6015229f 100644
---
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/PTransformTranslation.java
+++
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/PTransformTranslation.java
@@ -413,6 +413,7 @@ public String getUrn() {
}
@Nullable
+ @Override
public abstract RunnerApi.FunctionSpec getSpec();
public static UnknownRawPTransform forSpec(RunnerApi.FunctionSpec spec) {
diff --git
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/GreedyPipelineFuser.java
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/GreedyPipelineFuser.java
index aaa44b67c00..ea1dbae6a80 100644
---
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/GreedyPipelineFuser.java
+++
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/GreedyPipelineFuser.java
@@ -36,7 +36,6 @@
import java.util.NavigableSet;
import java.util.Queue;
import java.util.Set;
-import java.util.SortedSet;
import java.util.TreeSet;
import java.util.stream.Collectors;
import org.apache.beam.model.pipeline.v1.RunnerApi.Environment;
@@ -320,8 +319,9 @@ static DescendantConsumers of(
}
// Order sibling sets by their least siblings. This is stable across the
order siblings are
// generated, given stable IDs.
+ @SuppressWarnings("JdkObsolete")
NavigableSet<NavigableSet<CollectionConsumer>> orderedSiblings =
- new TreeSet<>(Comparator.comparing(SortedSet::first));
+ new TreeSet<>(Comparator.comparing(NavigableSet::first));
orderedSiblings.addAll(compatibleConsumers.values());
return orderedSiblings;
}
diff --git
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/Networks.java
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/Networks.java
index 508483395c2..9f41a6663f0 100644
---
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/Networks.java
+++
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/Networks.java
@@ -34,13 +34,11 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Queue;
import java.util.Set;
-import java.util.Stack;
import java.util.function.Function;
/** Static utility methods for {@link Network} instances that are directed. */
@@ -197,7 +195,7 @@ public final NodeT apply(NodeT input) {
// Linked hashset will prevent duplicates from appearing and will maintain
insertion order.
LinkedHashSet<NodeT> nodes = new LinkedHashSet<>(network.nodes().size());
- Queue<NodeT> processingOrder = new LinkedList<>();
+ Queue<NodeT> processingOrder = new ArrayDeque<>();
// Add all the roots
for (NodeT node : network.nodes()) {
if (network.inDegree(node) == 0) {
@@ -259,7 +257,7 @@ private static String escapeDot(String s) {
*/
public static <NodeT, EdgeT> List<List<NodeT>> allPathsFromRootsToLeaves(
Network<NodeT, EdgeT> network) {
- Stack<List<NodeT>> paths = new Stack<>();
+ ArrayDeque<List<NodeT>> paths = new ArrayDeque<>();
// Populate the list with all roots
for (NodeT node : network.nodes()) {
if (network.inDegree(node) == 0) {
@@ -268,14 +266,14 @@ private static String escapeDot(String s) {
}
List<List<NodeT>> distinctPathsFromRootsToLeaves = new ArrayList<>();
- while (!paths.empty()) {
- List<NodeT> path = paths.pop();
+ while (!paths.isEmpty()) {
+ List<NodeT> path = paths.removeFirst();
NodeT lastNode = path.get(path.size() - 1);
if (network.outDegree(lastNode) == 0) {
distinctPathsFromRootsToLeaves.add(new ArrayList<>(path));
} else {
for (EdgeT edge : network.outEdges(lastNode)) {
- paths.push(
+ paths.addFirst(
ImmutableList.<NodeT>builder()
.addAll(path)
.add(network.incidentNodes(edge).target())
diff --git
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/PipelineNode.java
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/PipelineNode.java
index 3829142058d..55e331a78e3 100644
---
a/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/PipelineNode.java
+++
b/runners/core-construction-java/src/main/java/org/apache/beam/runners/core/construction/graph/PipelineNode.java
@@ -41,7 +41,7 @@ static PCollectionNode pCollection(String id, PCollection
collection) {
*/
@AutoValue
abstract class PCollectionNode implements PipelineNode {
- public abstract String getId();
+ @Override public abstract String getId();
public abstract PCollection getPCollection();
}
@@ -50,7 +50,7 @@ static PCollectionNode pCollection(String id, PCollection
collection) {
*/
@AutoValue
abstract class PTransformNode implements PipelineNode {
- public abstract String getId();
+ @Override public abstract String getId();
public abstract PTransform getTransform();
}
}
diff --git
a/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ArtifactServiceStagerTest.java
b/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ArtifactServiceStagerTest.java
index a61ab9fcb83..06d5fa939c7 100644
---
a/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ArtifactServiceStagerTest.java
+++
b/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ArtifactServiceStagerTest.java
@@ -34,6 +34,7 @@
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
+import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.util.Arrays;
import java.util.Collections;
@@ -82,7 +83,7 @@ public void teardown() {
@Test
public void testStage() throws Exception {
File file = temp.newFile();
- byte[] content = "foo-bar-baz".getBytes();
+ byte[] content = "foo-bar-baz".getBytes(StandardCharsets.UTF_8);
byte[] contentMd5 = MessageDigest.getInstance("MD5").digest(content);
try (FileChannel contentChannel = new FileOutputStream(file).getChannel())
{
contentChannel.write(ByteBuffer.wrap(content));
@@ -106,19 +107,19 @@ public void testStage() throws Exception {
@Test
public void testStagingMultipleFiles() throws Exception {
File file = temp.newFile();
- byte[] content = "foo-bar-baz".getBytes();
+ byte[] content = "foo-bar-baz".getBytes(StandardCharsets.UTF_8);
try (FileChannel contentChannel = new FileOutputStream(file).getChannel())
{
contentChannel.write(ByteBuffer.wrap(content));
}
File otherFile = temp.newFile();
- byte[] otherContent = "spam-ham-eggs".getBytes();
+ byte[] otherContent = "spam-ham-eggs".getBytes(StandardCharsets.UTF_8);
try (FileChannel contentChannel = new
FileOutputStream(otherFile).getChannel()) {
contentChannel.write(ByteBuffer.wrap(otherContent));
}
File thirdFile = temp.newFile();
- byte[] thirdContent = "up, down, charm, top, bottom, strange".getBytes();
+ byte[] thirdContent = "up, down, charm, top, bottom,
strange".getBytes(StandardCharsets.UTF_8);
try (FileChannel contentChannel = new
FileOutputStream(thirdFile).getChannel()) {
contentChannel.write(ByteBuffer.wrap(thirdContent));
}
diff --git
a/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/CommonCoderTest.java
b/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/CommonCoderTest.java
index 6bfc82d56fc..c2bf601005a 100644
---
a/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/CommonCoderTest.java
+++
b/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/CommonCoderTest.java
@@ -36,6 +36,7 @@
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import com.google.auto.value.AutoValue;
import com.google.common.base.MoreObjects;
+import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.CharStreams;
@@ -43,9 +44,9 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
@@ -148,9 +149,9 @@ static OneCoderTestSpec create(
// Would like to use the InputStream directly with Jackson, but Jackson
does not seem to
// support streams of multiple entities. Instead, read the entire YAML as
a String and split
// it up manually, passing each to Jackson.
- String specString = CharStreams.toString(new InputStreamReader(stream));
- String[] specs = specString.split("\n---\n");
- List<OneCoderTestSpec> ret = new LinkedList<>();
+ String specString = CharStreams.toString(new InputStreamReader(stream,
StandardCharsets.UTF_8));
+ Iterable<String> specs = Splitter.on("\n---\n").split(specString);
+ List<OneCoderTestSpec> ret = new ArrayList<>();
for (String spec : specs) {
CommonCoderTestSpec coderTestSpec = parseSpec(spec);
CommonCoder coder = coderTestSpec.getCoder();
@@ -224,7 +225,7 @@ private static Object convertValue(Object value,
CommonCoder coderSpec, Coder co
} else if (s.equals(getUrn(StandardCoders.Enum.ITERABLE))) {
Coder elementCoder = ((IterableCoder) coder).getElemCoder();
List<Object> elements = (List<Object>) value;
- List<Object> convertedElements = new LinkedList<>();
+ List<Object> convertedElements = new ArrayList<>();
for (Object element : elements) {
convertedElements.add(
convertValue(element, coderSpec.getComponents().get(0),
elementCoder));
@@ -239,7 +240,7 @@ private static Object convertValue(Object value,
CommonCoder coderSpec, Coder co
Object windowValue = convertValue(
kvMap.get("value"), coderSpec.getComponents().get(0), valueCoder);
Instant timestamp = new Instant(((Number)
kvMap.get("timestamp")).longValue());
- List<BoundedWindow> windows = new LinkedList<>();
+ List<BoundedWindow> windows = new ArrayList<>();
for (Object window : ((List<Object>) kvMap.get("windows"))) {
windows.add((BoundedWindow) convertValue(window,
coderSpec.getComponents().get(1),
windowCoder));
@@ -258,7 +259,7 @@ private static Object convertValue(Object value,
CommonCoder coderSpec, Coder co
}
private static Coder<?> instantiateCoder(CommonCoder coder) {
- List<Coder<?>> components = new LinkedList<>();
+ List<Coder<?>> components = new ArrayList<>();
for (CommonCoder innerCoder : coder.getComponents()) {
components.add(instantiateCoder(innerCoder));
}
diff --git
a/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ReadTranslationTest.java
b/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ReadTranslationTest.java
index 3783e083510..648bd4784b5 100644
---
a/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ReadTranslationTest.java
+++
b/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/ReadTranslationTest.java
@@ -157,7 +157,7 @@ public int hashCode() {
return TestUnboundedSource.class.hashCode();
}
- private class TestCheckpointMarkCoder extends AtomicCoder<CheckpointMark> {
+ private static class TestCheckpointMarkCoder extends
AtomicCoder<CheckpointMark> {
@Override
public void encode(CheckpointMark value, OutputStream outStream)
throws CoderException, IOException {
diff --git
a/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/graph/ProtoOverridesTest.java
b/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/graph/ProtoOverridesTest.java
index 988e4652af1..5e945da8070 100644
---
a/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/graph/ProtoOverridesTest.java
+++
b/runners/core-construction-java/src/test/java/org/apache/beam/runners/core/construction/graph/ProtoOverridesTest.java
@@ -26,6 +26,7 @@
import com.google.common.collect.ImmutableList;
import com.google.protobuf.ByteString;
+import java.nio.charset.StandardCharsets;
import org.apache.beam.model.pipeline.v1.RunnerApi;
import org.apache.beam.model.pipeline.v1.RunnerApi.AccumulationMode.Enum;
import org.apache.beam.model.pipeline.v1.RunnerApi.Coder;
@@ -79,7 +80,8 @@ public void replacesOnlyMatching() {
.setSpec(
FunctionSpec.newBuilder()
.setUrn("beam:second:replacement")
- .setPayload(ByteString.copyFrom("foo-bar-baz".getBytes())))
+ .setPayload(
+
ByteString.copyFrom("foo-bar-baz".getBytes(StandardCharsets.UTF_8))))
.build();
WindowingStrategy introducedWS =
WindowingStrategy.newBuilder().setAccumulationMode(Enum.ACCUMULATING).build();
@@ -150,7 +152,7 @@ public void replacesMultiple() {
Coder.newBuilder().setSpec(SdkFunctionSpec.getDefaultInstance()).build()))
.build();
- ByteString newPayload = ByteString.copyFrom("foo-bar-baz".getBytes());
+ ByteString newPayload =
ByteString.copyFrom("foo-bar-baz".getBytes(StandardCharsets.UTF_8));
Pipeline updated =
ProtoOverrides.updateTransform(
"beam:repeated",
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 105939)
Time Spent: 20m (was: 10m)
> Enforce ErrorProne analysis in runners-core-construction project
> ----------------------------------------------------------------
>
> Key: BEAM-4307
> URL: https://issues.apache.org/jira/browse/BEAM-4307
> Project: Beam
> Issue Type: Improvement
> Components: runner-core
> Reporter: Scott Wegner
> Assignee: Ismaël Mejía
> Priority: Minor
> Labels: errorprone, starter
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Java ErrorProne static analysis was [recently
> enabled|https://github.com/apache/beam/pull/5161] in the Gradle build
> process, but only as warnings. ErrorProne errors are generally useful and
> easy to fix. Some work was done to [make sdks-java-core
> ErrorProne-clean|https://github.com/apache/beam/pull/5319] and add
> enforcement. This task is clean ErrorProne warnings and add enforcement in
> {{beam-runners-core-construction-java}}. Additional context discussed on the
> [dev
> list|https://lists.apache.org/thread.html/95aae2785c3cd728c2d3378cbdff2a7ba19caffcd4faa2049d2e2f46@%3Cdev.beam.apache.org%3E].
> Fixing this issue will involve:
> # Follow instructions in the [Contribution
> Guide|https://beam.apache.org/contribute/] to set up a {{beam}} development
> environment.
> # Run the following command to compile and run ErrorProne analysis on the
> project: {{./gradlew :beam-runners-core-construction-java:assemble}}
> # Fix each ErrorProne warning from the {{runners/core-construction}} project.
> # In {{runners/core-construction/build.gradle}}, add {{failOnWarning: true}}
> to the call the {{applyJavaNature()}}
> ([example|https://github.com/apache/beam/pull/5319/files#diff-9390c20635aed5f42f83b97506a87333R20]).
> This starter issue is sponsored by [~swegner]. Feel free to [reach
> out|https://beam.apache.org/community/contact-us/] with questions or code
> review:
> * JIRA: [~swegner]
> * GitHub: [@swegner|https://github.com/swegner]
> * Slack: [@Scott Wegner|https://s.apache.org/beam-slack-channel]
> * Email: swegner at google dot com
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
