You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2023/06/08 23:28:00 UTC
[calcite] branch main updated (7dc94e381b -> c0e6ba264b)
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
from 7dc94e381b [CALCITE-5757] BigQuery DATE_TRUNC return type should be ARG0 and TIMESTAMP_TRUNC/DATETIME_TRUNC should return TIMESTAMP for DATE/TIMESTAMPs and TIMESTAMP_LTZ for TIMESTAMP_LTZ
new 644a3f0727 [CALCITE-5762] Create class TestUnsafe, that contains unsafe methods used by tests
new 4219c993b6 [CALCITE-5706] Add class PairList
new 267326d15e [CALCITE-5764] Add Puffin
new ac4920bb0a In Puffin, add actions before and after each source and all sources
new abf05e39ee Code style: Lint
new c0e6ba264b [CALCITE-5765] Add LintTest, to apply custom lint rules to source code
The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
build.gradle.kts | 4 +-
.../apache/calcite/plan/SubstitutionVisitor.java | 2 +-
.../calcite/plan/volcano/IterativeRuleDriver.java | 2 +-
.../org/apache/calcite/rex/RexVisitorImpl.java | 4 +-
.../java/org/apache/calcite/runtime/PairList.java | 178 ++++++++++
.../apache/calcite/sql/SqlDescriptorOperator.java | 2 +-
.../java/org/apache/calcite/util/Glossary.java | 2 +-
.../main/java/org/apache/calcite/util/Puffin.java | 392 +++++++++++++++++++++
.../main/java/org/apache/calcite/util/Sources.java | 18 +-
.../apache/calcite/rex/RexProgramBuilderBase.java | 2 +-
.../apache/calcite/sql/test/DocumentationTest.java | 32 +-
.../java/org/apache/calcite/test/JdbcTest.java | 3 +-
.../java/org/apache/calcite/test/LintTest.java | 283 +++++++++++++++
.../java/org/apache/calcite/test/PuffinTest.java | 174 +++++++++
.../org/apache/calcite/test/SqlValidatorTest.java | 6 +-
.../concurrent/ConcurrentTestCommandScript.java | 85 +----
.../java/org/apache/calcite/util/PairListTest.java | 126 +++++++
.../java/org/apache/calcite/util/TestUnsafe.java | 210 +++++++++++
.../calcite/util/graph/DirectedGraphTest.java | 2 +-
.../calcite/adapter/elasticsearch/Scrolling.java | 2 +-
.../apache/calcite/linq4j/EnumerableDefaults.java | 4 +-
.../apache/calcite/linq4j/QueryableDefaults.java | 2 +-
.../org/apache/calcite/linq4j/tree/Blocks.java | 2 +-
.../org/apache/calcite/linq4j/tree/Expression.java | 2 +-
.../org/apache/calcite/linq4j/tree/Statement.java | 2 +-
.../java/org/apache/calcite/util/TestUtil.java | 41 ++-
26 files changed, 1442 insertions(+), 140 deletions(-)
create mode 100644 core/src/main/java/org/apache/calcite/runtime/PairList.java
create mode 100644 core/src/main/java/org/apache/calcite/util/Puffin.java
create mode 100644 core/src/test/java/org/apache/calcite/test/LintTest.java
create mode 100644 core/src/test/java/org/apache/calcite/test/PuffinTest.java
create mode 100644 core/src/test/java/org/apache/calcite/util/PairListTest.java
create mode 100644 core/src/test/java/org/apache/calcite/util/TestUnsafe.java
[calcite] 04/06: In Puffin, add actions before and after each source and all sources
Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit ac4920bb0ab52a25c3b2cf008f361ff0fbc7f82a
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Thu Jun 8 00:20:49 2023 -0700
In Puffin, add actions before and after each source and all sources
---
.../main/java/org/apache/calcite/util/Puffin.java | 80 +++++++++++++++++++---
.../java/org/apache/calcite/test/PuffinTest.java | 74 ++++++++++++++++++--
2 files changed, 140 insertions(+), 14 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/util/Puffin.java b/core/src/main/java/org/apache/calcite/util/Puffin.java
index f776d58030..3be201ea4e 100644
--- a/core/src/main/java/org/apache/calcite/util/Puffin.java
+++ b/core/src/main/java/org/apache/calcite/util/Puffin.java
@@ -78,7 +78,8 @@ public class Puffin {
public static <G, F> Builder<G, F> builder(Supplier<G> globalStateFactory,
Function<G, F> fileStateFactory) {
return new BuilderImpl<>(globalStateFactory, fileStateFactory,
- PairList.of(), new ArrayList<>());
+ PairList.of(), new ArrayList<>(), new ArrayList<>(), new ArrayList<>(),
+ new ArrayList<>());
}
/** Creates a Builder with no state. */
@@ -92,9 +93,23 @@ public class Puffin {
* @param <F> Type of state that is created when we start processing a file
* @see Puffin#builder */
public interface Builder<G, F> {
+ /** Adds a predicate and action to be invoked on each line of a source. */
Builder<G, F> add(Predicate<Line<G, F>> linePredicate,
Consumer<Line<G, F>> action);
+
+ /** Adds an action to be called before each source. */
+ Builder<G, F> beforeSource(Consumer<Context<G, F>> action);
+
+ /** Adds an action to be called after each source. */
+ Builder<G, F> afterSource(Consumer<Context<G, F>> action);
+
+ /** Adds an action to be called before all sources. */
+ Builder<G, F> before(Consumer<Context<G, F>> action);
+
+ /** Adds an action to be called after all sources. */
Builder<G, F> after(Consumer<Context<G, F>> action);
+
+ /** Builds the program. */
Program<G> build();
}
@@ -239,8 +254,11 @@ public class Puffin {
private static class ProgramImpl<G, F> implements Program<G> {
private final Supplier<G> globalStateFactory;
private final Function<G, F> fileStateFactory;
- private final PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> pairList;
- private final ImmutableList<Consumer<Context<G, F>>> endList;
+ private final PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> onLineList;
+ private final ImmutableList<Consumer<Context<G, F>>> beforeSourceList;
+ private final ImmutableList<Consumer<Context<G, F>>> afterSourceList;
+ private final ImmutableList<Consumer<Context<G, F>>> beforeList;
+ private final ImmutableList<Consumer<Context<G, F>>> afterList;
@SuppressWarnings("Convert2MethodRef")
private final LoadingCache<String, Pattern> patternCache0 =
CacheBuilder.newBuilder()
@@ -250,18 +268,31 @@ public class Puffin {
private ProgramImpl(Supplier<G> globalStateFactory,
Function<G, F> fileStateFactory,
- PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> pairList,
- ImmutableList<Consumer<Context<G, F>>> endList) {
+ PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> onLineList,
+ ImmutableList<Consumer<Context<G, F>>> beforeSourceList,
+ ImmutableList<Consumer<Context<G, F>>> afterSourceList,
+ ImmutableList<Consumer<Context<G, F>>> beforeList,
+ ImmutableList<Consumer<Context<G, F>>> afterList) {
this.globalStateFactory = globalStateFactory;
this.fileStateFactory = fileStateFactory;
- this.pairList = pairList;
- this.endList = endList;
+ this.onLineList = onLineList;
+ this.beforeSourceList = beforeSourceList;
+ this.afterSourceList = afterSourceList;
+ this.beforeList = beforeList;
+ this.afterList = afterList;
}
@Override public G execute(Stream<? extends Source> sources,
PrintWriter out) {
final G globalState = globalStateFactory.get();
+ final Source source0 = Sources.of("");
+ final F fileState0 = fileStateFactory.apply(globalState);
+ final ContextImpl<G, F> x0 =
+ new ContextImpl<G, F>(out, source0, patternCache, globalState,
+ fileState0);
+ beforeList.forEach(action -> action.accept(x0));
sources.forEach(source -> execute(globalState, source, out));
+ afterList.forEach(action -> action.accept(x0));
return globalState;
}
@@ -272,20 +303,21 @@ public class Puffin {
final ContextImpl<G, F> x =
new ContextImpl<G, F>(out, source, patternCache, globalState,
fileState);
+ beforeSourceList.forEach(action -> action.accept(x));
for (;;) {
String lineText = br.readLine();
if (lineText == null) {
- endList.forEach(end -> end.accept(x));
break;
}
++x.fnr;
x.line = lineText;
- pairList.forEach((predicate, action) -> {
+ onLineList.forEach((predicate, action) -> {
if (predicate.test(x)) {
action.accept(x);
}
});
}
+ afterSourceList.forEach(action -> action.accept(x));
} catch (IOException e) {
throw new RuntimeException(e);
}
@@ -301,15 +333,24 @@ public class Puffin {
private final Supplier<G> globalStateFactory;
private final Function<G, F> fileStateFactory;
final PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> onLineList;
+ final List<Consumer<Context<G, F>>> beforeSourceList;
+ final List<Consumer<Context<G, F>>> afterSourceList;
+ final List<Consumer<Context<G, F>>> beforeList;
final List<Consumer<Context<G, F>>> afterList;
private BuilderImpl(Supplier<G> globalStateFactory,
Function<G, F> fileStateFactory,
PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> onLineList,
+ List<Consumer<Context<G, F>>> beforeSourceList,
+ List<Consumer<Context<G, F>>> afterSourceList,
+ List<Consumer<Context<G, F>>> beforeList,
List<Consumer<Context<G, F>>> afterList) {
this.globalStateFactory = globalStateFactory;
this.fileStateFactory = fileStateFactory;
this.onLineList = onLineList;
+ this.beforeSourceList = beforeSourceList;
+ this.afterSourceList = afterSourceList;
+ this.beforeList = beforeList;
this.afterList = afterList;
}
@@ -319,6 +360,21 @@ public class Puffin {
return this;
}
+ @Override public Builder<G, F> beforeSource(Consumer<Context<G, F>> action) {
+ beforeSourceList.add(action);
+ return this;
+ }
+
+ @Override public Builder<G, F> afterSource(Consumer<Context<G, F>> action) {
+ afterSourceList.add(action);
+ return this;
+ }
+
+ @Override public Builder<G, F> before(Consumer<Context<G, F>> action) {
+ beforeList.add(action);
+ return this;
+ }
+
@Override public Builder<G, F> after(Consumer<Context<G, F>> action) {
afterList.add(action);
return this;
@@ -326,7 +382,11 @@ public class Puffin {
@Override public Program<G> build() {
return new ProgramImpl<>(globalStateFactory, fileStateFactory,
- onLineList.immutable(), ImmutableList.copyOf(afterList));
+ onLineList.immutable(),
+ ImmutableList.copyOf(beforeSourceList),
+ ImmutableList.copyOf(afterSourceList),
+ ImmutableList.copyOf(beforeList),
+ ImmutableList.copyOf(afterList));
}
}
}
diff --git a/core/src/test/java/org/apache/calcite/test/PuffinTest.java b/core/src/test/java/org/apache/calcite/test/PuffinTest.java
index 604685e0eb..3cbebca6a8 100644
--- a/core/src/test/java/org/apache/calcite/test/PuffinTest.java
+++ b/core/src/test/java/org/apache/calcite/test/PuffinTest.java
@@ -26,14 +26,19 @@ import org.junit.jupiter.api.Test;
import java.io.PrintWriter;
import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
import java.util.stream.Stream;
import static org.apache.calcite.test.Matchers.isLinux;
+import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.hasToString;
/** Tests {@link Puffin}. */
@@ -42,19 +47,71 @@ public class PuffinTest {
new Fixture<>(Sources.of(""), Puffin.builder().build());
@Test void testPuffin() {
- Puffin.Program<Unit> program =
- Puffin.builder(() -> Unit.INSTANCE, u -> new AtomicInteger())
+ Puffin.Program<AtomicInteger> program =
+ Puffin.builder(AtomicInteger::new, counter -> Unit.INSTANCE)
.add(line -> !line.startsWith("#")
&& !line.matches(".*/\\*.*\\*/.*"),
- line -> line.state().incrementAndGet())
+ line -> line.globalState().incrementAndGet())
.after(context ->
- context.println("counter: " + context.state().get()))
+ context.println("counter: " + context.globalState().get()))
.build();
fixture().withDefaultInput()
.withProgram(program)
.generatesOutput(isLinux("counter: 2\n"));
}
+ /** Tests Puffin with several sources, registers actions by calling
+ * {@link Puffin.Builder#beforeSource(Consumer)},
+ * {@link Puffin.Builder#afterSource(Consumer)},
+ * {@link Puffin.Builder#before(Consumer)}, and
+ * {@link Puffin.Builder#after(Consumer)}, and counts how many times each is
+ * called. */
+ @Test void testSeveralSources() {
+ Puffin.Program<GlobalState> program =
+ Puffin.builder(GlobalState::new, u -> new AtomicInteger())
+ .add(line -> true,
+ line -> line.state().incrementAndGet())
+ .beforeSource(context -> {
+ final GlobalState g = context.globalState();
+ g.beforeSourceCount.incrementAndGet();
+ })
+ .afterSource(context -> {
+ final GlobalState g = context.globalState();
+ final AtomicInteger f = context.state();
+ g.messages.add(f.intValue() + " lines");
+ g.afterSourceCount.incrementAndGet();
+ })
+ .before(context -> {
+ final GlobalState g = context.globalState();
+ g.beforeCount.incrementAndGet();
+ })
+ .after(context -> {
+ final GlobalState g = context.globalState();
+ g.afterCount.incrementAndGet();
+ g.messages.add(g.afterSourceCount + " after sources");
+ g.messages.add(g.beforeSourceCount + " before sources");
+ g.messages.add(g.beforeCount + " before");
+ g.messages.add(g.afterCount + " after");
+ })
+ .build();
+ final StringWriter sw = new StringWriter();
+ GlobalState g =
+ program.execute(
+ Stream.of(Sources.of("a\nb\n"),
+ Sources.of("a\n"),
+ Sources.of("a\nb\nc\n")),
+ new PrintWriter(sw));
+ assertThat(g.messages, hasSize(7));
+ assertThat(g.messages, hasItem("3 lines"));
+ assertThat(g.messages, hasItem("2 lines"));
+ assertThat(g.messages, hasItem("1 lines"));
+ assertThat(g.messages, hasItem("3 after sources"));
+ assertThat(g.messages, hasItem("3 before sources"));
+ assertThat(g.messages, hasItem("1 before"));
+ assertThat(g.messages, hasItem("1 after"));
+ assertThat(sw, hasToString(""));
+ }
+
@Test void testEmptyProgram() {
final Puffin.Program<Unit> program = Puffin.builder().build();
fixture().withDefaultInput()
@@ -105,4 +162,13 @@ public class PuffinTest {
return this;
}
}
+
+ /** Global state. */
+ private static class GlobalState {
+ final List<String> messages = new ArrayList<>();
+ final AtomicInteger beforeSourceCount = new AtomicInteger();
+ final AtomicInteger afterSourceCount = new AtomicInteger();
+ final AtomicInteger beforeCount = new AtomicInteger();
+ final AtomicInteger afterCount = new AtomicInteger();
+ }
}
[calcite] 03/06: [CALCITE-5764] Add Puffin
Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 267326d15e347df18af816127b5e5cef6a8bf3d4
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Fri Jun 2 22:37:52 2023 -0700
[CALCITE-5764] Add Puffin
Puffin provides a similar programming model to Awk. A program
is a collection of rules, each of which is a predicate
applied to a line of text followed by an action on that test.
Puffin programs are thread-safe; if Puffin is invoked on
mutiple sources (files), each file is processed in its own
thread and is allocated its own state that the rules can use
without coordination.
Puffin caches compiled regular expressions, so that calls to
`matches(String)` are almost as efficient as if the program
had called `Pattern.compile(String)` in advance.
---
.../main/java/org/apache/calcite/util/Puffin.java | 332 +++++++++++++++++++++
.../main/java/org/apache/calcite/util/Sources.java | 18 +-
.../java/org/apache/calcite/test/PuffinTest.java | 108 +++++++
3 files changed, 452 insertions(+), 6 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/util/Puffin.java b/core/src/main/java/org/apache/calcite/util/Puffin.java
new file mode 100644
index 0000000000..f776d58030
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/util/Puffin.java
@@ -0,0 +1,332 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.util;
+
+import org.apache.calcite.runtime.PairList;
+import org.apache.calcite.runtime.Unit;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.collect.ImmutableList;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.PrintWriter;
+import java.io.Reader;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.function.Supplier;
+import java.util.regex.Pattern;
+import java.util.stream.Stream;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A text processor similar to Awk.
+ *
+ * <p>Example use:
+ *
+ * <blockquote><pre>{@code
+ * File file;
+ * final Puffin.Program program =
+ * Puffin.builder()
+ * .add(line -> !line.startsWith("#"),
+ * line -> counter.incrementAndGet())
+ * .after(context ->
+ * context.println("There were " + counter.get()
+ * + " uncommented lines"))
+ * .build();
+ * program.execute(Source.of(file), System.out);
+ * }</pre></blockquote>
+ *
+ * <p>prints the following to stdout:
+ *
+ * <blockquote>{@code
+ * There were 3 uncommented lines.
+ * }</blockquote>
+ */
+public class Puffin {
+ private Puffin() {
+ }
+
+ /** Creates a Builder.
+ *
+ * @param fileStateFactory Creates the state for each file
+ * @return Builder
+ * @param <G> Type of state that is created when we start processing
+ * @param <F> Type of state that is created when we start processing a file
+ */
+ public static <G, F> Builder<G, F> builder(Supplier<G> globalStateFactory,
+ Function<G, F> fileStateFactory) {
+ return new BuilderImpl<>(globalStateFactory, fileStateFactory,
+ PairList.of(), new ArrayList<>());
+ }
+
+ /** Creates a Builder with no state. */
+ public static Builder<Unit, Unit> builder() {
+ return builder(() -> Unit.INSTANCE, u -> u);
+ }
+
+ /** Fluent interface for constructing a Program.
+ *
+ * @param <G> Type of state that is created when we start processing
+ * @param <F> Type of state that is created when we start processing a file
+ * @see Puffin#builder */
+ public interface Builder<G, F> {
+ Builder<G, F> add(Predicate<Line<G, F>> linePredicate,
+ Consumer<Line<G, F>> action);
+ Builder<G, F> after(Consumer<Context<G, F>> action);
+ Program<G> build();
+ }
+
+ /** A Puffin program. You can execute it on a file.
+ *
+ * @param <G> Type of state that is created when we start processing */
+ public interface Program<G> {
+ /** Executes this program. */
+ G execute(Stream<? extends Source> sources, PrintWriter out);
+
+ /** Executes this program, writing to an output stream such as
+ * {@link System#out}. */
+ default void execute(Stream<? extends Source> sources, OutputStream out) {
+ try (PrintWriter w = Util.printWriter(out)) {
+ execute(sources, w);
+ }
+ }
+
+ /** Executes this program on a single source. */
+ default void execute(Source source, OutputStream out) {
+ execute(Stream.of(source), out);
+ }
+ }
+
+ /** A line in a file.
+ *
+ * <p>Created by an executing program and passed to the predicate
+ * and action that you registered in
+ * {@link Builder#add(Predicate, Consumer)}.
+ *
+ * @param <G> Type of state that is created when we start processing
+ * @param <F> Type of state that is created when we start processing a file
+ */
+ public interface Line<G, F> {
+ G globalState();
+ F state();
+ int fnr();
+ Source source();
+ boolean startsWith(String prefix);
+ boolean contains(CharSequence s);
+ boolean endsWith(String suffix);
+ boolean matches(String regex);
+ String line();
+ }
+
+ /** Context for executing a Puffin program within a given file.
+ *
+ * @param <G> Type of state that is created when we start processing
+ * @param <F> Type of state that is created when we start processing a file */
+ public static class Context<G, F> {
+ final PrintWriter out;
+ final Source source;
+ final F fileState;
+ final G globalState;
+ private final Function<String, Pattern> patternCache;
+
+ /** Holds the current line. */
+ String line = "";
+
+ /** Holds the current line number in the file (starting from 1).
+ *
+ * <p>Corresponds to the Awk variable {@code FNR}, which stands for "file
+ * number of records". */
+ int fnr = 0;
+
+ Context(PrintWriter out, Source source,
+ Function<String, Pattern> patternCache, G globalState,
+ F fileState) {
+ this.out = requireNonNull(out, "out");
+ this.source = requireNonNull(source, "source");
+ this.patternCache = requireNonNull(patternCache, "patternCache");
+ this.globalState = requireNonNull(globalState, "globalState");
+ this.fileState = requireNonNull(fileState, "fileState");
+ }
+
+ public F state() {
+ return fileState;
+ }
+
+ public G globalState() {
+ return globalState;
+ }
+
+ public void println(String s) {
+ out.println(s);
+ }
+
+ Pattern pattern(String regex) {
+ return patternCache.apply(regex);
+ }
+ }
+
+ /** Extension to {@link Context} that also implements {@link Line}.
+ *
+ * <p>We don't want clients to know that {@code Context} implements
+ * {@code Line}, but neither do we want to create a new {@code Line} object
+ * for every line in the file. Making this a subclass accomplishes both
+ * goals.
+ *
+ * @param <G> Type of state that is created when we start processing
+ * @param <F> Type of state that is created when we start processing a file */
+ static class ContextImpl<G, F> extends Context<G, F> implements Line<G, F> {
+
+ ContextImpl(PrintWriter out, Source source,
+ Function<String, Pattern> patternCache, G globalState, F state) {
+ super(out, source, patternCache, globalState, state);
+ }
+
+ @Override public int fnr() {
+ return fnr;
+ }
+
+ @Override public Source source() {
+ return source;
+ }
+
+ @Override public boolean startsWith(String prefix) {
+ return line.startsWith(prefix);
+ }
+
+ @Override public boolean contains(CharSequence s) {
+ return line.contains(s);
+ }
+
+ @Override public boolean endsWith(String suffix) {
+ return line.endsWith(suffix);
+ }
+
+ @Override public boolean matches(String regex) {
+ return pattern(regex).matcher(line).matches();
+ }
+
+ @Override public String line() {
+ return line;
+ }
+ }
+
+ /** Implementation of {@link Program}.
+ *
+ * @param <G> Type of state that is created when we start processing
+ * @param <F> Type of state that is created when we start processing a file */
+ private static class ProgramImpl<G, F> implements Program<G> {
+ private final Supplier<G> globalStateFactory;
+ private final Function<G, F> fileStateFactory;
+ private final PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> pairList;
+ private final ImmutableList<Consumer<Context<G, F>>> endList;
+ @SuppressWarnings("Convert2MethodRef")
+ private final LoadingCache<String, Pattern> patternCache0 =
+ CacheBuilder.newBuilder()
+ .build(CacheLoader.from(regex -> Pattern.compile(regex)));
+ private final Function<String, Pattern> patternCache =
+ patternCache0::getUnchecked;
+
+ private ProgramImpl(Supplier<G> globalStateFactory,
+ Function<G, F> fileStateFactory,
+ PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> pairList,
+ ImmutableList<Consumer<Context<G, F>>> endList) {
+ this.globalStateFactory = globalStateFactory;
+ this.fileStateFactory = fileStateFactory;
+ this.pairList = pairList;
+ this.endList = endList;
+ }
+
+ @Override public G execute(Stream<? extends Source> sources,
+ PrintWriter out) {
+ final G globalState = globalStateFactory.get();
+ sources.forEach(source -> execute(globalState, source, out));
+ return globalState;
+ }
+
+ private void execute(G globalState, Source source, PrintWriter out) {
+ try (Reader r = source.reader();
+ BufferedReader br = new BufferedReader(r)) {
+ final F fileState = fileStateFactory.apply(globalState);
+ final ContextImpl<G, F> x =
+ new ContextImpl<G, F>(out, source, patternCache, globalState,
+ fileState);
+ for (;;) {
+ String lineText = br.readLine();
+ if (lineText == null) {
+ endList.forEach(end -> end.accept(x));
+ break;
+ }
+ ++x.fnr;
+ x.line = lineText;
+ pairList.forEach((predicate, action) -> {
+ if (predicate.test(x)) {
+ action.accept(x);
+ }
+ });
+ }
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+ }
+
+
+ /** Implementation of Builder.
+ *
+ * @param <G> Type of state that is created when we start processing
+ * @param <F> Type of state that is created when we start processing a file */
+ private static class BuilderImpl<G, F> implements Builder<G, F> {
+ private final Supplier<G> globalStateFactory;
+ private final Function<G, F> fileStateFactory;
+ final PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> onLineList;
+ final List<Consumer<Context<G, F>>> afterList;
+
+ private BuilderImpl(Supplier<G> globalStateFactory,
+ Function<G, F> fileStateFactory,
+ PairList<Predicate<Line<G, F>>, Consumer<Line<G, F>>> onLineList,
+ List<Consumer<Context<G, F>>> afterList) {
+ this.globalStateFactory = globalStateFactory;
+ this.fileStateFactory = fileStateFactory;
+ this.onLineList = onLineList;
+ this.afterList = afterList;
+ }
+
+ @Override public Builder<G, F> add(Predicate<Line<G, F>> linePredicate,
+ Consumer<Line<G, F>> action) {
+ onLineList.add(linePredicate, action);
+ return this;
+ }
+
+ @Override public Builder<G, F> after(Consumer<Context<G, F>> action) {
+ afterList.add(action);
+ return this;
+ }
+
+ @Override public Program<G> build() {
+ return new ProgramImpl<>(globalStateFactory, fileStateFactory,
+ onLineList.immutable(), ImmutableList.copyOf(afterList));
+ }
+ }
+}
diff --git a/core/src/main/java/org/apache/calcite/util/Sources.java b/core/src/main/java/org/apache/calcite/util/Sources.java
index 86cd859ad1..205a0fdfd9 100644
--- a/core/src/main/java/org/apache/calcite/util/Sources.java
+++ b/core/src/main/java/org/apache/calcite/util/Sources.java
@@ -23,7 +23,6 @@ import com.google.common.io.CharSource;
import org.checkerframework.checker.nullness.qual.Nullable;
import java.io.File;
-import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
@@ -33,6 +32,7 @@ import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Locale;
import java.util.Objects;
@@ -62,12 +62,18 @@ public abstract class Sources {
}
}
- /**
- * Create {@link Source} from a generic text source such as string, {@link java.nio.CharBuffer}
- * or text file. Useful when data is already in memory or can't be directly read from
+ /** Creates a {@link Source} from a character sequence such as a
+ * {@link String}. */
+ public static Source of(CharSequence s) {
+ return fromCharSource(CharSource.wrap(s));
+ }
+
+ /** Creates a {@link Source} from a generic text source such as string,
+ * {@link java.nio.CharBuffer} or text file. Useful when data is already
+ * in memory or can't be directly read from
* a file or url.
*
- * @param source generic "re-redable" source of characters
+ * @param source generic "re-readable" source of characters
* @return {@code Source} delegate for {@code CharSource} (can't be null)
* @throws NullPointerException when {@code source} is null
*/
@@ -276,7 +282,7 @@ public abstract class Sources {
@Override public InputStream openStream() throws IOException {
if (file != null) {
- return new FileInputStream(file);
+ return Files.newInputStream(file.toPath());
} else {
return url.openStream();
}
diff --git a/core/src/test/java/org/apache/calcite/test/PuffinTest.java b/core/src/test/java/org/apache/calcite/test/PuffinTest.java
new file mode 100644
index 0000000000..604685e0eb
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/test/PuffinTest.java
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.test;
+
+import org.apache.calcite.runtime.Unit;
+import org.apache.calcite.util.Puffin;
+import org.apache.calcite.util.Source;
+import org.apache.calcite.util.Sources;
+
+import org.hamcrest.Matcher;
+import org.junit.jupiter.api.Test;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Stream;
+
+import static org.apache.calcite.test.Matchers.isLinux;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasToString;
+
+/** Tests {@link Puffin}. */
+public class PuffinTest {
+ private static final Fixture<Unit> EMPTY_FIXTURE =
+ new Fixture<>(Sources.of(""), Puffin.builder().build());
+
+ @Test void testPuffin() {
+ Puffin.Program<Unit> program =
+ Puffin.builder(() -> Unit.INSTANCE, u -> new AtomicInteger())
+ .add(line -> !line.startsWith("#")
+ && !line.matches(".*/\\*.*\\*/.*"),
+ line -> line.state().incrementAndGet())
+ .after(context ->
+ context.println("counter: " + context.state().get()))
+ .build();
+ fixture().withDefaultInput()
+ .withProgram(program)
+ .generatesOutput(isLinux("counter: 2\n"));
+ }
+
+ @Test void testEmptyProgram() {
+ final Puffin.Program<Unit> program = Puffin.builder().build();
+ fixture().withDefaultInput()
+ .withProgram(program)
+ .generatesOutput(is(""));
+ }
+
+ static Fixture<Unit> fixture() {
+ return EMPTY_FIXTURE;
+ }
+
+ /** Fixture that contains all the state necessary to test
+ * {@link Puffin}.
+ *
+ * @param <G> Type of state that is created when we start processing */
+ private static class Fixture<G> {
+ private final Source source;
+ private final Puffin.Program<G> program;
+
+ Fixture(Source source, Puffin.Program<G> program) {
+ this.source = source;
+ this.program = program;
+ }
+
+ public Fixture<G> withDefaultInput() {
+ final String inputText = "first line\n"
+ + "# second line\n"
+ + "third line /* with a comment */\n"
+ + "fourth line";
+ return withSource(Sources.of(inputText));
+ }
+
+ private Fixture<G> withSource(Source source) {
+ return new Fixture<>(source, program);
+ }
+
+ public <G2> Fixture<G2> withProgram(Puffin.Program<G2> program) {
+ return new Fixture<>(source, program);
+ }
+
+ public Fixture<G> generatesOutput(Matcher<String> matcher) {
+ StringWriter sw = new StringWriter();
+ try (PrintWriter pw = new PrintWriter(sw)) {
+ G g = program.execute(Stream.of(source), pw);
+ assertThat(g, notNullValue());
+ }
+ assertThat(sw, hasToString(matcher));
+ return this;
+ }
+ }
+}
[calcite] 06/06: [CALCITE-5765] Add LintTest, to apply custom lint rules to source code
Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit c0e6ba264b3e522010a982ed35de9f3dd03be6af
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Sun Jun 4 11:24:38 2023 -0700
[CALCITE-5765] Add LintTest, to apply custom lint rules to source code
LintTest.testLint applies rules to Calcite's java source files,
most of them concerned with the formatting of javadoc comments,
and fails if any of the rules give warnings.
LintTest.testLog verifies recent git commit messages. The
goal is to prevent malformed messages that begin with a
space, end in a '.', or contain the word 'fix'. (Many commit
messages that contain the word 'fix' are describing the code
change; they should instead describe the problem seen by the
user, which is what will be most useful to those reading the
release notes. If the message describes the problem, the
word 'fix' is implicit.)
Also create method `TestUtil.getBase(File)`, based on some
logic previously in `DocumentationTest`.
Close apache/calcite#3250
---
.../apache/calcite/sql/test/DocumentationTest.java | 32 +--
.../java/org/apache/calcite/test/LintTest.java | 283 +++++++++++++++++++++
.../java/org/apache/calcite/util/TestUnsafe.java | 97 +++++++
.../java/org/apache/calcite/util/TestUtil.java | 41 ++-
4 files changed, 421 insertions(+), 32 deletions(-)
diff --git a/core/src/test/java/org/apache/calcite/sql/test/DocumentationTest.java b/core/src/test/java/org/apache/calcite/sql/test/DocumentationTest.java
index 5085a38a68..dda80bc7b0 100644
--- a/core/src/test/java/org/apache/calcite/sql/test/DocumentationTest.java
+++ b/core/src/test/java/org/apache/calcite/sql/test/DocumentationTest.java
@@ -27,7 +27,7 @@ import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.parser.SqlAbstractParserImpl;
import org.apache.calcite.sql.parser.SqlParserTest;
import org.apache.calcite.test.DiffTestCase;
-import org.apache.calcite.util.Sources;
+import org.apache.calcite.util.TestUtil;
import org.apache.calcite.util.Util;
import org.junit.jupiter.api.Test;
@@ -50,7 +50,6 @@ import java.util.stream.Collectors;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.fail;
/** Various automated checks on the documentation. */
class DocumentationTest {
@@ -189,35 +188,8 @@ class DocumentationTest {
final File inFile;
final File outFile;
- private boolean isProjectDir(File dir) {
- return new File(dir, "pom.xml").isFile()
- || new File(dir, "build.gradle.kts").isFile()
- || new File(dir, "gradle.properties").isFile();
- }
-
FileFixture() {
- // Algorithm:
- // 1) Find location of DocumentationTest.class
- // 2) Climb via getParentFile() until we detect pom.xml
- // 3) It means we've got core/pom.xml, and we need to get core/../site/
- Class<DocumentationTest> klass = DocumentationTest.class;
- File docTestClass =
- Sources.of(klass.getResource(klass.getSimpleName() + ".class")).file();
-
- File core = docTestClass.getAbsoluteFile();
- for (int i = 0; i < 42; i++) {
- if (isProjectDir(core)) {
- // Ok, core == core/
- break;
- }
- core = core.getParentFile();
- }
- if (!isProjectDir(core)) {
- fail("Unable to find neither core/pom.xml nor core/build.gradle.kts. Started with "
- + docTestClass.getAbsolutePath()
- + ", the current path is " + core.getAbsolutePath());
- }
- base = core.getParentFile();
+ base = TestUtil.getBaseDir(DocumentationTest.class);
inFile = new File(base, "site/_docs/reference.md");
// TODO: replace with core/build/ when Maven is migrated to Gradle
// It does work in Gradle, however, we don't want to create "target" folder in Gradle
diff --git a/core/src/test/java/org/apache/calcite/test/LintTest.java b/core/src/test/java/org/apache/calcite/test/LintTest.java
new file mode 100644
index 0000000000..d84ea99dc0
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/test/LintTest.java
@@ -0,0 +1,283 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.test;
+
+import org.apache.calcite.util.Puffin;
+import org.apache.calcite.util.Source;
+import org.apache.calcite.util.Sources;
+import org.apache.calcite.util.TestUnsafe;
+import org.apache.calcite.util.Util;
+
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Consumer;
+import java.util.function.Function;
+import java.util.stream.Stream;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+
+/** Various automated checks on the code and git history. */
+class LintTest {
+ @SuppressWarnings("Convert2MethodRef") // JDK 8 requires lambdas
+ private Puffin.Program<GlobalState> makeProgram() {
+ return Puffin.builder(GlobalState::new, global -> new FileState(global))
+ .add(line -> line.fnr() == 1,
+ line -> line.globalState().fileCount++)
+
+ // Javadoc does not require '</p>', so we do not allow '</p>'
+ .add(line -> line.state().inJavadoc()
+ && line.contains("</p>"),
+ line -> line.state().message("no '</p>'", line))
+
+ // No "**/"
+ .add(line -> line.contains("**/")
+ && line.state().inJavadoc(),
+ line ->
+ line.state().message("no '**/'; use '*/'",
+ line))
+
+ // A Javadoc paragraph '<p>' must not be on its own line.
+ .add(line -> line.matches("^ *\\* <p>"),
+ line ->
+ line.state().message("<p> must not be on its own line",
+ line))
+
+ // A Javadoc paragraph '<p>' must be preceded by a blank Javadoc
+ // line.
+ .add(line -> line.matches("^ *\\*"),
+ line -> line.state().starLine = line.fnr())
+ .add(line -> line.matches("^ *\\* <p>.*")
+ && line.fnr() - 1 != line.state().starLine,
+ line ->
+ line.state().message("<p> must be preceded by blank line",
+ line))
+
+ // The first "@param" of a javadoc block must be preceded by a blank
+ // line.
+ .add(line -> line.matches("^ */\\*\\*.*"),
+ line -> line.state().javadocStartLine = line.fnr())
+ .add(line -> line.matches(".*\\*/"),
+ line -> line.state().javadocEndLine = line.fnr())
+ .add(line -> line.matches("^ *\\* @.*"),
+ line -> {
+ if (line.state().inJavadoc()
+ && line.state().atLine < line.state().javadocStartLine
+ && line.fnr() - 1 != line.state().starLine) {
+ line.state().message(
+ "First @tag must be preceded by blank line",
+ line);
+ }
+ line.state().atLine = line.fnr();
+ })
+ .build();
+ }
+
+ @Test void testProgramWorks() {
+ final String code = "class MyClass {\n"
+ + " /** Paragraph.\n"
+ + " *\n"
+ + " * Missing p.\n"
+ + " *\n"
+ + " * <p>\n"
+ + " * <p>A paragraph (p must be preceded by blank line).\n"
+ + " *\n"
+ + " * <p>no p</p>\n"
+ + " * @see java.lang.String (should be preceded by blank line)\n"
+ + " **/\n"
+ + " String x = \"ok because it's not in javadoc:</p>\";\n"
+ + "}\n";
+ final String expectedMessages = "["
+ + "GuavaCharSource{memory}:6:"
+ + "<p> must not be on its own line\n"
+ + "GuavaCharSource{memory}:7:"
+ + "<p> must be preceded by blank line\n"
+ + "GuavaCharSource{memory}:9:"
+ + "no '</p>'\n"
+ + "GuavaCharSource{memory}:10:"
+ + "First @tag must be preceded by blank line\n"
+ + "GuavaCharSource{memory}:11:"
+ + "no '**/'; use '*/']";
+ final Puffin.Program<GlobalState> program = makeProgram();
+ final StringWriter sw = new StringWriter();
+ final GlobalState g;
+ try (PrintWriter pw = new PrintWriter(sw)) {
+ g = program.execute(Stream.of(Sources.of(code)), pw);
+ }
+ assertThat(g.messages.toString().replace(", ", "\n"),
+ is(expectedMessages));
+ }
+
+ /** Tests that source code has no flaws. */
+ @Test void testLint() {
+ assumeTrue(TestUnsafe.haveGit(), "Invalid git environment");
+
+ final Puffin.Program<GlobalState> program = makeProgram();
+ final List<File> javaFiles = TestUnsafe.getJavaFiles();
+
+ final GlobalState g;
+ try (PrintWriter pw = Util.printWriter(System.out)) {
+ g = program.execute(javaFiles.parallelStream().map(Sources::of), pw);
+ }
+
+ g.messages.forEach(System.out::println);
+ assertThat(g.messages, empty());
+ }
+
+ /** Tests that the most recent N commit messages are good.
+ *
+ * <p>N needs to be large enough to verify multi-commit PRs, but not so large
+ * that it fails because of historical commits. */
+ @Test void testLintLog() {
+ assumeTrue(TestUnsafe.haveGit(), "Invalid git environment");
+
+ int n = 7;
+ final List<String> messages = TestUnsafe.getCommitMessages(n);
+ final List<String> warnings = new ArrayList<>();
+ for (String message : messages) {
+ checkMessage(message, warning ->
+ warnings.add("invalid git log message '" + message + "'; "
+ + warning));
+ }
+ warnings.forEach(System.out::println);
+ assertThat(warnings, empty());
+ }
+
+ @Test void testLogMatcher() {
+ final Function<String, List<String>> f = message -> {
+ final List<String> warnings = new ArrayList<>();
+ checkMessage(message, warnings::add);
+ return warnings;
+ };
+ assertThat(f.apply(" [CALCITE-1234] abc"),
+ hasItem("starts with space"));
+ assertThat(f.apply("[CALCITE-1234] abc"),
+ hasItem("malformed [CALCITE-nnnn] reference"));
+ assertThat(f.apply("[CALCITE-12b] abc"),
+ hasItem("malformed [CALCITE-nnnn] reference"));
+ assertThat(f.apply("[CALCITE-12345] abc"),
+ hasItem("malformed [CALCITE-nnnn] reference"));
+ assertThat(f.apply("[CALCITE-1234]: abc"),
+ hasItem("malformed [CALCITE-nnnn] reference"));
+ assertThat(f.apply("CALCITE-1234: abc"),
+ hasItem("malformed [CALCITE-nnnn] reference"));
+ assertThat(f.apply("[CALCITE-12] abc"),
+ empty());
+ assertThat(f.apply("[CALCITE-123] abc"),
+ empty());
+ assertThat(f.apply("[CALCITE-1234] Fix problem with foo"),
+ hasItem("contains 'fix' or 'fixes'; you should describe the "
+ + "problem, not what you did"));
+ assertThat(f.apply("[CALCITE-1234] Baz doesn't buzz"),
+ empty());
+ assertThat(f.apply("[CALCITE-1234] Baz doesn't buzz."),
+ hasItem("ends with period"));
+ assertThat(f.apply("[CALCITE-1234] Two problems."),
+ hasSize(2));
+ assertThat(f.apply("[CALCITE-1234] Two problems."),
+ hasItem("ends with period"));
+ assertThat(f.apply("[CALCITE-1234] Two problems."),
+ hasItem("malformed [CALCITE-nnnn] reference"));
+ assertThat(f.apply("Cosmetic: Move everything one character to the left"),
+ empty());
+ assertThat(
+ f.apply("Finishing up [CALCITE-4937], remove workarounds for "
+ + "[CALCITE-4877]"),
+ empty());
+ assertThat(f.apply("Fix typo in filterable-model.yaml"),
+ empty());
+ assertThat(f.apply("Fix typo in filterable-model.yaml"),
+ empty());
+ assertThat(f.apply("Revert \"[CALCITE-4817] Expand SubstitutionVisitor\""),
+ empty());
+ }
+
+ private static void checkMessage(String message, Consumer<String> consumer) {
+ if (message.startsWith(" ")) {
+ consumer.accept("starts with space");
+ }
+ if (message.endsWith(".")) {
+ consumer.accept("ends with period");
+ }
+ if (message.endsWith(" ")) {
+ consumer.accept("ends with space");
+ }
+ if (message.startsWith("[CALCITE-")
+ || message.startsWith("CALCITE-")) {
+ if (!message.matches("^\\[CALCITE-[0-9]{1,4}] [^ ].*")) {
+ consumer.accept("malformed [CALCITE-nnnn] reference");
+ }
+ if (message.matches("(?i).*\\b(fix|fixes)\\b.*")) {
+ consumer.accept("contains 'fix' or 'fixes'; you should describe the "
+ + "problem, not what you did");
+ }
+ }
+ }
+
+ /** Warning that code is not as it should be. */
+ private static class Message {
+ final Source source;
+ final int line;
+ final String message;
+
+ Message(Source source, int line, String message) {
+ this.source = source;
+ this.line = line;
+ this.message = message;
+ }
+
+ @Override public String toString() {
+ return source + ":" + line + ":" + message;
+ }
+ }
+
+ /** Internal state of the lint rules. */
+ private static class GlobalState {
+ int fileCount = 0;
+ final List<Message> messages = new ArrayList<>();
+ }
+
+ /** Internal state of the lint rules, per file. */
+ private static class FileState {
+ final GlobalState global;
+ int starLine;
+ int atLine;
+ int javadocStartLine;
+ int javadocEndLine;
+
+ FileState(GlobalState global) {
+ this.global = global;
+ }
+
+ void message(String message, Puffin.Line<GlobalState, FileState> line) {
+ global.messages.add(new Message(line.source(), line.fnr(), message));
+ }
+
+ public boolean inJavadoc() {
+ return javadocEndLine < javadocStartLine;
+ }
+ }
+}
diff --git a/core/src/test/java/org/apache/calcite/util/TestUnsafe.java b/core/src/test/java/org/apache/calcite/util/TestUnsafe.java
index e0d2f8f7ce..89e6395aac 100644
--- a/core/src/test/java/org/apache/calcite/util/TestUnsafe.java
+++ b/core/src/test/java/org/apache/calcite/util/TestUnsafe.java
@@ -16,16 +16,21 @@
*/
package org.apache.calcite.util;
+import com.google.common.collect.ImmutableList;
+
import org.checkerframework.checker.nullness.qual.Nullable;
import org.slf4j.Logger;
import java.io.BufferedInputStream;
import java.io.BufferedOutputStream;
+import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.Reader;
+import java.io.StringReader;
+import java.io.StringWriter;
import java.io.Writer;
import java.util.List;
@@ -110,4 +115,96 @@ public abstract class TestUnsafe {
}
return status;
}
+
+ /** Returns whether we seem are in a valid environment. */
+ public static boolean haveGit() {
+ // Is there a '.git' directory? If not, we may be in a source tree
+ // unzipped from a tarball.
+ final File base = TestUtil.getBaseDir(TestUnsafe.class);
+ final File gitDir = new File(base, ".git");
+ if (!gitDir.exists()
+ || !gitDir.isDirectory()
+ || !gitDir.canRead()) {
+ return false;
+ }
+
+ // Execute a simple git command. If it fails, we're probably not in a
+ // valid git environment.
+ final List<String> argumentList =
+ ImmutableList.of("git", "--version");
+ try {
+ final StringWriter sw = new StringWriter();
+ int status =
+ runAppProcess(argumentList, base, null, null, sw);
+ final String s = sw.toString();
+ if (status != 0) {
+ return false;
+ }
+ } catch (Exception e) {
+ return false;
+ }
+ return true;
+ }
+
+ /** Returns a list of Java files in git under a given directory.
+ *
+ * <p>Assumes running Linux or macOS, and that git is available. */
+ public static List<File> getJavaFiles() {
+ String s;
+ try {
+ final List<String> argumentList =
+ ImmutableList.of("git", "ls-files", "*.java");
+ final File base = TestUtil.getBaseDir(TestUnsafe.class);
+ try {
+ final StringWriter sw = new StringWriter();
+ int status =
+ runAppProcess(argumentList, base, null, null, sw);
+ if (status != 0) {
+ throw new RuntimeException("command " + argumentList
+ + ": exited with status " + status);
+ }
+ s = sw.toString();
+ } catch (Exception e) {
+ throw new RuntimeException("command " + argumentList
+ + ": failed with exception", e);
+ }
+
+ final ImmutableList.Builder<File> files = ImmutableList.builder();
+ try (StringReader r = new StringReader(s);
+ BufferedReader br = new BufferedReader(r)) {
+ for (;;) {
+ String line = br.readLine();
+ if (line == null) {
+ break;
+ }
+ files.add(new File(base, line));
+ }
+ }
+ return files.build();
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ /** Returns the messages of the {@code n} most recent commits. */
+ public static List<String> getCommitMessages(int n) {
+ final File base = TestUtil.getBaseDir(TestUnsafe.class);
+ final List<String> argumentList =
+ ImmutableList.of("git", "log", "-n" + n, "--pretty=format:%s");
+ try {
+ final StringWriter sw = new StringWriter();
+ int status =
+ runAppProcess(argumentList, base, null, null, sw);
+ String s = sw.toString();
+ if (status != 0) {
+ throw new RuntimeException("command " + argumentList
+ + ": exited with status " + status
+ + (s.isEmpty() ? "" : "; output [" + s + "]"));
+ }
+ return ImmutableList.copyOf(s.split("\n"));
+ } catch (Exception e) {
+ throw new RuntimeException("command " + argumentList
+ + ": failed with exception", e);
+ }
+ }
}
diff --git a/testkit/src/main/java/org/apache/calcite/util/TestUtil.java b/testkit/src/main/java/org/apache/calcite/util/TestUtil.java
index 68f753d6b1..a313fbd604 100644
--- a/testkit/src/main/java/org/apache/calcite/util/TestUtil.java
+++ b/testkit/src/main/java/org/apache/calcite/util/TestUtil.java
@@ -22,16 +22,21 @@ import com.google.common.collect.ImmutableSortedSet;
import org.junit.jupiter.api.Assertions;
+import java.io.File;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.reflect.InvocationTargetException;
+import java.net.URL;
import java.util.List;
-import java.util.Objects;
import java.util.SortedSet;
import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import static java.util.Objects.requireNonNull;
+
/**
* Static utilities for JUnit tests.
*/
@@ -269,7 +274,7 @@ public abstract class TestUtil {
*/
@VisibleForTesting
static int majorVersionFromString(String version) {
- Objects.requireNonNull(version, "version");
+ requireNonNull(version, "version");
if (version.startsWith("1.")) {
// running on version <= 8 (expecting string of type: x.y.z*)
@@ -310,6 +315,38 @@ public abstract class TestUtil {
return System.getProperty("java.vm.vendor");
}
+ /** Returns the root directory of the source tree. */
+ public static File getBaseDir(Class<?> klass) {
+ // Algorithm:
+ // 1) Find location of TestUtil.class
+ // 2) Climb via getParentFile() until we detect pom.xml
+ // 3) It means we've got BASE/testkit/pom.xml, and we need to get BASE
+ final URL resource = klass.getResource(klass.getSimpleName() + ".class");
+ final File classFile =
+ Sources.of(requireNonNull(resource, "resource")).file();
+
+ File file = classFile.getAbsoluteFile();
+ for (int i = 0; i < 42; i++) {
+ if (isProjectDir(file)) {
+ // Ok, file == BASE/testkit/
+ break;
+ }
+ file = file.getParentFile();
+ }
+ if (!isProjectDir(file)) {
+ fail("Could not find pom.xml, build.gradle.kts or gradle.properties. "
+ + "Started with " + classFile.getAbsolutePath()
+ + ", the current path is " + file.getAbsolutePath());
+ }
+ return file.getParentFile();
+ }
+
+ private static boolean isProjectDir(File dir) {
+ return new File(dir, "pom.xml").isFile()
+ || new File(dir, "build.gradle.kts").isFile()
+ || new File(dir, "gradle.properties").isFile();
+ }
+
/** Given a list, returns the number of elements that are not between an
* element that is less and an element that is greater. */
public static <E extends Comparable<E>> SortedSet<E> outOfOrderItems(List<E> list) {
[calcite] 02/06: [CALCITE-5706] Add class PairList
Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 4219c993b642f7ba66f7b9a28a66f4616eb1168a
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Tue Feb 7 11:16:34 2023 -0800
[CALCITE-5706] Add class PairList
`PairList` is a list whose entries are pairs (`interface Map.Entry`).
Its implementation is efficient (backed by a single list,
where the left and right parts of each pair are stored in
even, odd positions).
In addition to the usual `List` methods, there are additional
methods for pairs: `add(K, V)`, `leftList()`, `rightList()`,
`toImmutableMap()`, `forEach(BiConsumer<K, V>)`,
`forEach(IndexedBiConsumer<K, V>)`,
By default `PairList` is immutable, but using the method
`PairList.immutable()` you can override the backing list and
create an immutable `PairList`.
---
.../java/org/apache/calcite/runtime/PairList.java | 178 +++++++++++++++++++++
.../java/org/apache/calcite/util/PairListTest.java | 126 +++++++++++++++
2 files changed, 304 insertions(+)
diff --git a/core/src/main/java/org/apache/calcite/runtime/PairList.java b/core/src/main/java/org/apache/calcite/runtime/PairList.java
new file mode 100644
index 0000000000..c3cfaf14e2
--- /dev/null
+++ b/core/src/main/java/org/apache/calcite/runtime/PairList.java
@@ -0,0 +1,178 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.runtime;
+
+import org.apache.calcite.util.Pair;
+import org.apache.calcite.util.Util;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.AbstractList;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.function.BiConsumer;
+
+import static java.util.Objects.requireNonNull;
+
+/** A list of pairs, stored as a quotient list.
+ *
+ * @param <T> First type
+ * @param <U> Second type
+ */
+public class PairList<T, U> extends AbstractList<Map.Entry<T, U>> {
+ final List<@Nullable Object> list;
+
+ private PairList(List<@Nullable Object> list) {
+ this.list = list;
+ }
+
+ /** Creates an empty PairList. */
+ public static <T, U> PairList<T, U> of() {
+ return new PairList<>(new ArrayList<>());
+ }
+
+ /** Creates a PairList backed by a given list.
+ *
+ * <p>Changes to the backing list will be reflected in the PairList.
+ * If the backing list is immutable, this PairList will be also. */
+ public static <T, U> PairList<T, U> backedBy(List<@Nullable Object> list) {
+ return new PairList<>(list);
+ }
+
+ /** Creates a PairList from a Map. */
+ @SuppressWarnings("RedundantCast")
+ public static <T, U> PairList<T, U> of(Map<T, U> map) {
+ final List<@Nullable Object> list = new ArrayList<>(map.size() * 2);
+ map.forEach((t, u) -> {
+ list.add((Object) t);
+ list.add((Object) u);
+ });
+ return new PairList<>(list);
+ }
+
+ @SuppressWarnings("unchecked")
+ @Override public Map.Entry<T, U> get(int index) {
+ int x = index * 2;
+ return Pair.of((T) list.get(x), (U) list.get(x + 1));
+ }
+
+ @Override public int size() {
+ return list.size() / 2;
+ }
+
+ @SuppressWarnings("RedundantCast")
+ @Override public boolean add(Map.Entry<T, U> entry) {
+ list.add((Object) entry.getKey());
+ list.add((Object) entry.getValue());
+ return true;
+ }
+
+ @SuppressWarnings("RedundantCast")
+ @Override public void add(int index, Map.Entry<T, U> entry) {
+ int x = index * 2;
+ list.add(x, (Object) entry.getKey());
+ list.add(x + 1, (Object) entry.getValue());
+ }
+
+ /** Adds a pair to this list. */
+ @SuppressWarnings("RedundantCast")
+ public void add(T t, U u) {
+ list.add((Object) t);
+ list.add((Object) u);
+ }
+
+ @SuppressWarnings("unchecked")
+ @Override public Map.Entry<T, U> remove(int index) {
+ final int x = index * 2;
+ T t = (T) list.remove(x);
+ U u = (U) list.remove(x);
+ return Pair.of(t, u);
+ }
+
+ /** Returns an unmodifiable list view consisting of the left entry of each
+ * pair. */
+ @SuppressWarnings("unchecked")
+ public List<T> leftList() {
+ return Util.quotientList((List<T>) list, 2, 0);
+ }
+
+ /** Returns an unmodifiable list view consisting of the right entry of each
+ * pair. */
+ @SuppressWarnings("unchecked")
+ public List<U> rightList() {
+ return Util.quotientList((List<U>) list, 2, 1);
+ }
+
+ /** Calls a BiConsumer with each pair in this list. */
+ @SuppressWarnings("unchecked")
+ public void forEach(BiConsumer<T, U> consumer) {
+ requireNonNull(consumer, "consumer");
+ for (int i = 0; i < list.size();) {
+ T t = (T) list.get(i++);
+ U u = (U) list.get(i++);
+ consumer.accept(t, u);
+ }
+ }
+
+ /** Calls a BiConsumer with each pair in this list. */
+ @SuppressWarnings("unchecked")
+ public void forEachIndexed(IndexedBiConsumer<T, U> consumer) {
+ requireNonNull(consumer, "consumer");
+ for (int i = 0, j = 0; i < list.size();) {
+ T t = (T) list.get(i++);
+ U u = (U) list.get(i++);
+ consumer.accept(j++, t, u);
+ }
+ }
+
+ /** Creates an {@link ImmutableMap} whose entries are the pairs in this list.
+ * Throws if keys are not unique. */
+ public ImmutableMap<T, U> toImmutableMap() {
+ final ImmutableMap.Builder<T, U> b = ImmutableMap.builder();
+ forEach((t, u) -> b.put(t, u));
+ return b.build();
+ }
+
+ /** Returns an immutable PairList whose contents are the same as this
+ * PairList. */
+ public PairList<T, U> immutable() {
+ final List<@Nullable Object> immutableList = ImmutableList.copyOf(list);
+ return backedBy(immutableList);
+ }
+
+ /** Action to be taken each step of an indexed iteration over a PairList.
+ *
+ * @param <T> First type
+ * @param <U> Second type
+ *
+ * @see PairList#forEachIndexed(IndexedBiConsumer)
+ */
+ public interface IndexedBiConsumer<T, U> {
+ /**
+ * Performs this operation on the given arguments.
+ *
+ * @param index Index
+ * @param t First input argument
+ * @param u Second input argument
+ */
+ void accept(int index, T t, U u);
+ }
+}
diff --git a/core/src/test/java/org/apache/calcite/util/PairListTest.java b/core/src/test/java/org/apache/calcite/util/PairListTest.java
new file mode 100644
index 0000000000..3029475a89
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/util/PairListTest.java
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.util;
+
+import org.apache.calcite.runtime.PairList;
+
+import com.google.common.collect.ImmutableMap;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/** Unit test for {@code PairList}. */
+class PairListTest {
+ /** Basic test for {@link PairList}. */
+ @Test void testPairList() {
+ final PairList<Integer, String> pairList = PairList.of();
+ final List<Pair<Integer, String>> list = new ArrayList<>();
+
+ final Runnable validator = () -> {
+ assertThat(pairList.isEmpty(), is(list.isEmpty()));
+ assertThat(pairList, hasSize(list.size()));
+ assertThat(pairList.leftList(), hasSize(list.size()));
+ assertThat(pairList.rightList(), hasSize(list.size()));
+ assertThat(pairList.leftList(), is(Pair.left(list)));
+ assertThat(pairList.rightList(), is(Pair.right(list)));
+
+ final List<Map.Entry<Integer, String>> list2 = new ArrayList<>(pairList);
+ assertThat(list2, is(list));
+
+ // Check PairList.forEach(Consumer)
+ list2.clear();
+ //noinspection UseBulkOperation
+ pairList.forEach(p -> list2.add(p));
+ assertThat(list2, is(list));
+
+ // Check PairList.forEach(BiConsumer)
+ list2.clear();
+ pairList.forEach((k, v) -> list2.add(Pair.of(k, v)));
+ assertThat(list2, is(list));
+
+ // Check PairList.forEachIndexed
+ list2.clear();
+ pairList.forEachIndexed((i, k, v) -> {
+ assertThat(i, is(list2.size()));
+ list2.add(Pair.of(k, v));
+ });
+ assertThat(list2, is(list));
+
+ final PairList<Integer, String> immutablePairList = pairList.immutable();
+ assertThat(immutablePairList, hasSize(list.size()));
+ assertThat(immutablePairList, is(list));
+ assertThrows(UnsupportedOperationException.class, () ->
+ immutablePairList.add(0, ""));
+ list2.clear();
+ immutablePairList.forEach((k, v) -> list2.add(Pair.of(k, v)));
+ assertThat(list2, is(list));
+ };
+
+ validator.run();
+
+ pairList.add(1, "a");
+ list.add(Pair.of(1, "a"));
+ validator.run();
+
+ pairList.add(Pair.of(2, "b"));
+ list.add(Pair.of(2, "b"));
+ validator.run();
+
+ pairList.add(0, Pair.of(3, "c"));
+ list.add(0, Pair.of(3, "c"));
+ validator.run();
+
+ Map.Entry<Integer, String> x = pairList.remove(1);
+ Pair<Integer, String> y = list.remove(1);
+ assertThat(x, is(y));
+ validator.run();
+
+ pairList.clear();
+ list.clear();
+ validator.run();
+ }
+
+ /** Tests {@link PairList#of(Map)} and {@link PairList#toImmutableMap()}. */
+ @Test void testPairListOfMap() {
+ final ImmutableMap<String, Integer> map = ImmutableMap.of("a", 1, "b", 2);
+ final PairList<String, Integer> list = PairList.of(map);
+ assertThat(list, hasSize(2));
+ assertThat(list.toString(), is("[<a, 1>, <b, 2>]"));
+
+ final ImmutableMap<String, Integer> map2 = list.toImmutableMap();
+ assertThat(map2, is(map));
+
+ // After calling toImmutableMap, you can modify the list and call
+ // toImmutableMap again.
+ list.add("c", 3);
+ assertThat(list.toString(), is("[<a, 1>, <b, 2>, <c, 3>]"));
+ final ImmutableMap<String, Integer> map3 = list.toImmutableMap();
+ assertThat(map3.toString(), is("{a=1, b=2, c=3}"));
+
+ final Map<String, Integer> emptyMap = ImmutableMap.of();
+ final PairList<String, Integer> emptyList = PairList.of(emptyMap);
+ assertThat(emptyList.isEmpty(), is(true));
+ }
+}
[calcite] 05/06: Code style: Lint
Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit abf05e39ee822bae2e5da6526aeb40bbbbe0ea2e
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Sun Jun 4 11:18:51 2023 -0700
Code style: Lint
Add an autostyle rule, and fix its one violation.
Fix lint violations detected by [CALCITE-5765] LintTest (to follow).
---
build.gradle.kts | 1 +
core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java | 2 +-
.../java/org/apache/calcite/plan/volcano/IterativeRuleDriver.java | 2 +-
core/src/main/java/org/apache/calcite/rex/RexVisitorImpl.java | 4 ++--
.../src/main/java/org/apache/calcite/sql/SqlDescriptorOperator.java | 2 +-
core/src/main/java/org/apache/calcite/util/Glossary.java | 2 +-
.../src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java | 2 +-
core/src/test/java/org/apache/calcite/test/JdbcTest.java | 3 +--
core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java | 6 ++----
.../test/java/org/apache/calcite/util/graph/DirectedGraphTest.java | 2 +-
.../java/org/apache/calcite/adapter/elasticsearch/Scrolling.java | 2 +-
.../src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java | 4 ++--
.../src/main/java/org/apache/calcite/linq4j/QueryableDefaults.java | 2 +-
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Blocks.java | 2 +-
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expression.java | 2 +-
linq4j/src/main/java/org/apache/calcite/linq4j/tree/Statement.java | 2 +-
16 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/build.gradle.kts b/build.gradle.kts
index 87d7b9399c..f618a99340 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -603,6 +603,7 @@ allprojects {
replace("hamcrest: anyOf", "org.hamcrest.core.AnyOf.anyOf", "org.hamcrest.CoreMatchers.anyOf")
replace("hamcrest: containsString", "org.hamcrest.core.StringContains.containsString", "org.hamcrest.CoreMatchers.containsString")
replace("hamcrest: CoreMatchers", "import org.hamcrest.CoreMatchers;", "import static org.hamcrest.CoreMatchers.anything;")
+ replace("hamcrest: empty", "org.hamcrest.collection.IsEmptyCollection.empty", "org.hamcrest.Matchers.empty")
replace("hamcrest: emptyArray", "org.hamcrest.collection.IsArrayWithSize.emptyArray", "org.hamcrest.Matchers.emptyArray")
replace("hamcrest: endsWidth", "org.hamcrest.core.StringEndsWith.endsWith", "org.hamcrest.CoreMatchers.endsWith")
replace("hamcrest: equalTo", "org.hamcrest.core.IsEqual.equalTo", "org.hamcrest.CoreMatchers.equalTo")
diff --git a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
index 9668f44625..53c03f810a 100644
--- a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
+++ b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
@@ -877,7 +877,7 @@ public class SubstitutionVisitor {
}
/**
- * <p>Applies this rule to a particular node in a query. The goal is
+ * Applies this rule to a particular node in a query. The goal is
* to convert {@code query} into {@code target}. Before the rule is
* invoked, Calcite has made sure that query's children are equivalent
* to target's children.
diff --git a/core/src/main/java/org/apache/calcite/plan/volcano/IterativeRuleDriver.java b/core/src/main/java/org/apache/calcite/plan/volcano/IterativeRuleDriver.java
index a43a500db4..9aea576e95 100644
--- a/core/src/main/java/org/apache/calcite/plan/volcano/IterativeRuleDriver.java
+++ b/core/src/main/java/org/apache/calcite/plan/volcano/IterativeRuleDriver.java
@@ -22,7 +22,7 @@ import org.apache.calcite.util.trace.CalciteTrace;
import org.slf4j.Logger;
/***
- * <p>The algorithm executes repeatedly. The exact rules
+ * The algorithm executes repeatedly. The exact rules
* that may be fired varies.
*
* <p>The planner iterates over the rule matches presented
diff --git a/core/src/main/java/org/apache/calcite/rex/RexVisitorImpl.java b/core/src/main/java/org/apache/calcite/rex/RexVisitorImpl.java
index a0a45f5229..4f1fd8d63e 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexVisitorImpl.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexVisitorImpl.java
@@ -119,7 +119,7 @@ public class RexVisitorImpl<@Nullable R> implements RexVisitor<R> {
}
/**
- * <p>Visits an array of expressions, returning the logical 'and' of their
+ * Visits an array of expressions, returning the logical 'and' of their
* results.
*
* <p>If any of them returns false, returns false immediately; if they all
@@ -141,7 +141,7 @@ public class RexVisitorImpl<@Nullable R> implements RexVisitor<R> {
}
/**
- * <p>Visits an array of expressions, returning the logical 'or' of their
+ * Visits an array of expressions, returning the logical 'or' of their
* results.
*
* <p>If any of them returns true, returns true immediately; if they all
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlDescriptorOperator.java b/core/src/main/java/org/apache/calcite/sql/SqlDescriptorOperator.java
index 90cc7cccef..ef88e6691a 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlDescriptorOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlDescriptorOperator.java
@@ -25,7 +25,7 @@ import org.apache.calcite.sql.validate.SqlValidatorScope;
import static org.apache.calcite.util.Static.RESOURCE;
/**
- * <p>DESCRIPTOR appears as an argument in a function. DESCRIPTOR accepts a list of
+ * DESCRIPTOR appears as an argument in a function. DESCRIPTOR accepts a list of
* identifiers that represent a list of names. The interpretation of names is left
* to the function.
*
diff --git a/core/src/main/java/org/apache/calcite/util/Glossary.java b/core/src/main/java/org/apache/calcite/util/Glossary.java
index 3ee9c8989c..ddd6a723ef 100644
--- a/core/src/main/java/org/apache/calcite/util/Glossary.java
+++ b/core/src/main/java/org/apache/calcite/util/Glossary.java
@@ -29,7 +29,7 @@ public interface Glossary {
// CHECKSTYLE: OFF
/**
- * <p>This table shows how and where the Gang of Four patterns are applied.
+ * This table shows how and where the Gang of Four patterns are applied.
* The table uses information from the GoF book and from a course on
* advanced object design taught by Craig Larman.
*
diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java b/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java
index bc2b51da29..7fc52d9efa 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexProgramBuilderBase.java
@@ -251,7 +251,7 @@ public abstract class RexProgramBuilderBase {
* <p>Tries to expand the cast, and therefore the result may be something
* other than a {@link RexCall} to the CAST operator, such as a
* {@link RexLiteral}.
-
+ *
* @param e input node
* @param type type to cast to
* @return input node converted to given type
diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
index ad06270d49..2367c88699 100644
--- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java
+++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -474,8 +474,7 @@ public class JdbcTest {
connection.close();
}
- /**
- * <p>Test case for
+ /** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-3423">[CALCITE-3423]
* Support using CAST operation and BOOLEAN type value in table macro</a>. */
@Test void testTableMacroWithCastOrBoolean() throws SQLException {
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index c1df80bd00..1ae294a41f 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -106,10 +106,8 @@ public class SqlValidatorTest extends SqlValidatorTestCase {
//~ Static fields/initializers ---------------------------------------------
// CHECKSTYLE: IGNORE 1
- /**
- * @deprecated Deprecated so that usages of this constant will show up in
- * yellow in Intellij and maybe someone will fix them.
- */
+ /** @deprecated Deprecated so that usages of this constant will show up in
+ * yellow in Intellij and maybe someone will fix them. */
protected static final boolean TODO = false;
private static final String ANY = "(?s).*";
diff --git a/core/src/test/java/org/apache/calcite/util/graph/DirectedGraphTest.java b/core/src/test/java/org/apache/calcite/util/graph/DirectedGraphTest.java
index 2c4c6a27f7..1425c40dd0 100644
--- a/core/src/test/java/org/apache/calcite/util/graph/DirectedGraphTest.java
+++ b/core/src/test/java/org/apache/calcite/util/graph/DirectedGraphTest.java
@@ -33,9 +33,9 @@ import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.hasToString;
-import static org.hamcrest.collection.IsEmptyCollection.empty;
import static org.hamcrest.collection.IsIterableWithSize.iterableWithSize;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/Scrolling.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/Scrolling.java
index 573ddc2ca5..a75c061df8 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/Scrolling.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/Scrolling.java
@@ -27,7 +27,7 @@ import java.util.Objects;
import java.util.function.Consumer;
/**
- * <p>"Iterator" which retrieves results lazily and in batches. Uses
+ * "Iterator" which retrieves results lazily and in batches. Uses
* <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-scroll.html">Elastic Scrolling API</a>
* to optimally consume large search results.
*
diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java b/linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
index 9bf171e5a4..ae92498dc2 100644
--- a/linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
+++ b/linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
@@ -321,7 +321,7 @@ public abstract class EnumerableDefaults {
}
/**
- * <p>Analogous to LINQ's Enumerable.Cast extension method.
+ * Analogous to LINQ's Enumerable.Cast extension method.
*
* @param clazz Target type
* @param <T2> Target type
@@ -1644,7 +1644,7 @@ public abstract class EnumerableDefaults {
}
/**
- * <p>Fetches blocks of size {@code batchSize} from {@code outer},
+ * Fetches blocks of size {@code batchSize} from {@code outer},
* storing each block into a list ({@code outerValues}).
* For each block, it uses the {@code inner} function to
* obtain an enumerable with the correlated rows from the right (inner) input.
diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/QueryableDefaults.java b/linq4j/src/main/java/org/apache/calcite/linq4j/QueryableDefaults.java
index f84c085343..e3d96a3aec 100644
--- a/linq4j/src/main/java/org/apache/calcite/linq4j/QueryableDefaults.java
+++ b/linq4j/src/main/java/org/apache/calcite/linq4j/QueryableDefaults.java
@@ -218,7 +218,7 @@ public abstract class QueryableDefaults {
}
/**
- * <p>Analogous to LINQ's Enumerable.Cast extension method.
+ * Analogous to LINQ's Enumerable.Cast extension method.
*
* @param clazz Target type
* @param <T2> Target type
diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Blocks.java b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Blocks.java
index 578767adb6..b1a47c7d55 100644
--- a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Blocks.java
+++ b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Blocks.java
@@ -19,7 +19,7 @@ package org.apache.calcite.linq4j.tree;
import static java.util.Objects.requireNonNull;
/**
- * <p>Helper methods concerning {@link BlockStatement}s.
+ * Helper methods concerning {@link BlockStatement}s.
*
* @see BlockBuilder
*/
diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expression.java b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expression.java
index 15eb8bff9f..8354484c74 100644
--- a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expression.java
+++ b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Expression.java
@@ -19,7 +19,7 @@ package org.apache.calcite.linq4j.tree;
import java.lang.reflect.Type;
/**
- * <p>Analogous to LINQ's System.Linq.Expression.
+ * Analogous to LINQ's System.Linq.Expression.
*/
public abstract class Expression extends AbstractNode {
diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Statement.java b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Statement.java
index 8897ca11be..18648a4fc0 100644
--- a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Statement.java
+++ b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Statement.java
@@ -19,7 +19,7 @@ package org.apache.calcite.linq4j.tree;
import java.lang.reflect.Type;
/**
- * <p>Statement.
+ * Statement.
*/
public abstract class Statement extends AbstractNode {
protected Statement(ExpressionType nodeType, Type type) {
[calcite] 01/06: [CALCITE-5762] Create class TestUnsafe, that contains unsafe methods used by tests
Posted by jh...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 644a3f0727038e955791ad068c01d6dffc27a34d
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Wed Jun 7 11:00:15 2023 -0700
[CALCITE-5762] Create class TestUnsafe, that contains unsafe methods used by tests
---
build.gradle.kts | 3 +-
.../concurrent/ConcurrentTestCommandScript.java | 85 ++--------------
.../java/org/apache/calcite/util/TestUnsafe.java | 113 +++++++++++++++++++++
3 files changed, 120 insertions(+), 81 deletions(-)
diff --git a/build.gradle.kts b/build.gradle.kts
index dd45718913..87d7b9399c 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -761,8 +761,7 @@ allprojects {
"**/org/apache/calcite/adapter/os/Processes${'$'}ProcessFactory.class",
"**/org/apache/calcite/adapter/os/OsAdapterTest.class",
"**/org/apache/calcite/runtime/Resources${'$'}Inst.class",
- "**/org/apache/calcite/test/concurrent/ConcurrentTestCommandScript.class",
- "**/org/apache/calcite/test/concurrent/ConcurrentTestCommandScript${'$'}ShellCommand.class",
+ "**/org/apache/calcite/util/TestUnsafe.class",
"**/org/apache/calcite/util/Unsafe.class",
"**/org/apache/calcite/test/Unsafe.class"
)
diff --git a/core/src/test/java/org/apache/calcite/test/concurrent/ConcurrentTestCommandScript.java b/core/src/test/java/org/apache/calcite/test/concurrent/ConcurrentTestCommandScript.java
index e0e96aa6dd..b9e6136037 100644
--- a/core/src/test/java/org/apache/calcite/test/concurrent/ConcurrentTestCommandScript.java
+++ b/core/src/test/java/org/apache/calcite/test/concurrent/ConcurrentTestCommandScript.java
@@ -17,24 +17,17 @@
package org.apache.calcite.test.concurrent;
import org.apache.calcite.jdbc.SqlTimeoutException;
+import org.apache.calcite.util.TestUnsafe;
import org.apache.calcite.util.Unsafe;
import org.apache.calcite.util.Util;
-import org.slf4j.Logger;
-
-import java.io.BufferedInputStream;
-import java.io.BufferedOutputStream;
import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.File;
import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
import java.io.PrintWriter;
-import java.io.Reader;
import java.io.StringReader;
import java.io.StringWriter;
-import java.io.Writer;
import java.lang.reflect.Constructor;
import java.sql.Connection;
import java.sql.DriverManager;
@@ -279,68 +272,6 @@ public class ConcurrentTestCommandScript
return chars;
}
- /**
- * Runs an external application process.
- *
- * @param pb ProcessBuilder for the application
- * @param logger if not null, command and exit status will be logged here
- * @param appInput if not null, data will be copied to application's stdin
- * @param appOutput if not null, data will be captured from application's
- * stdout and stderr
- * @return application process exit value
- */
- static int runAppProcess(
- ProcessBuilder pb,
- Logger logger,
- Reader appInput,
- Writer appOutput) throws IOException, InterruptedException {
- pb.redirectErrorStream(true);
- if (logger != null) {
- logger.info("start process: " + pb.command());
- }
- Process p = pb.start();
-
- // Setup the input/output streams to the subprocess.
- // The buffering here is arbitrary. Javadocs strongly encourage
- // buffering, but the size needed is very dependent on the
- // specific application being run, the size of the input
- // provided by the caller, and the amount of output expected.
- // Since this method is currently used only by unit tests,
- // large-ish fixed buffer sizes have been chosen. If this
- // method becomes used for something in production, it might
- // be better to have the caller provide them as arguments.
- if (appInput != null) {
- OutputStream out =
- new BufferedOutputStream(
- p.getOutputStream(),
- 100 * 1024);
- int c;
- while ((c = appInput.read()) != -1) {
- out.write(c);
- }
- out.flush();
- }
- if (appOutput != null) {
- InputStream in =
- new BufferedInputStream(
- p.getInputStream(),
- 100 * 1024);
- int c;
- while ((c = in.read()) != -1) {
- appOutput.write(c);
- }
- appOutput.flush();
- in.close();
- }
- p.waitFor();
-
- int status = p.exitValue();
- if (logger != null) {
- logger.info("exit status=" + status + " from " + pb.command());
- }
- return status;
- }
-
/**
* Gets ready to execute: loads script FILENAME applying external variable
* BINDINGS.
@@ -1561,17 +1492,13 @@ public class ConcurrentTestCommandScript
Integer threadId = executor.getThreadId();
storeMessage(threadId, command);
- // argv[0] is found on $PATH. Working directory is the script's home
- // directory.
- //
- // WARNING: ProcessBuilder is security-sensitive. Its use is currently
- // safe because this code is under "core/test". Developers must not move
- // this code into "core/main".
- ProcessBuilder pb = new ProcessBuilder(argv);
- pb.directory(scriptDirectory);
try {
+ // argv[0] is found on $PATH.
+ // Working directory is the script's home directory.
// direct stdout & stderr to the threadWriter
- int status = runAppProcess(pb, null, null, getThreadWriter(threadId));
+ int status =
+ TestUnsafe.runAppProcess(argv, scriptDirectory, null, null,
+ getThreadWriter(threadId));
if (status != 0) {
storeMessage(threadId,
"command " + command + ": exited with status " + status);
diff --git a/core/src/test/java/org/apache/calcite/util/TestUnsafe.java b/core/src/test/java/org/apache/calcite/util/TestUnsafe.java
new file mode 100644
index 0000000000..e0d2f8f7ce
--- /dev/null
+++ b/core/src/test/java/org/apache/calcite/util/TestUnsafe.java
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.util;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+import org.slf4j.Logger;
+
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.Reader;
+import java.io.Writer;
+import java.util.List;
+
+/**
+ * Unsafe methods to be used by tests.
+ *
+ * <p>Contains methods that call JDK methods that the
+ * <a href="https://github.com/policeman-tools/forbidden-apis">forbidden
+ * APIs checker</a> does not approve of.
+ *
+ * <p>This class is excluded from the check, so methods called via this class
+ * will not fail the build.
+ *
+ * <p>Why is this in {@code core/src/test} and not in {@code testkit/src/main}?
+ * Because some of the methods (e.g. {@link #runAppProcess}) are so unsafe that
+ * they must not be on the class-path of production code.
+ */
+public abstract class TestUnsafe {
+ /**
+ * Runs an external application process.
+ *
+ * @param argumentList command name and its arguments
+ * @param directory working directory
+ * @param logger if not null, command and exit status will be logged here
+ * @param appInput if not null, data will be copied to application's stdin
+ * @param appOutput if not null, data will be captured from application's
+ * stdout and stderr
+ * @return application process exit value
+ */
+ public static int runAppProcess(List<String> argumentList, File directory,
+ @Nullable Logger logger, @Nullable Reader appInput,
+ @Nullable Writer appOutput) throws IOException, InterruptedException {
+
+ // WARNING: ProcessBuilder is security-sensitive. Its use is currently
+ // safe because this code is under "core/test". Developers must not move
+ // this code into "core/main".
+ final ProcessBuilder pb = new ProcessBuilder(argumentList);
+ pb.directory(directory);
+ pb.redirectErrorStream(true);
+ if (logger != null) {
+ logger.info("start process: " + pb.command());
+ }
+ Process p = pb.start();
+
+ // Setup the input/output streams to the subprocess.
+ // The buffering here is arbitrary. Javadocs strongly encourage
+ // buffering, but the size needed is very dependent on the
+ // specific application being run, the size of the input
+ // provided by the caller, and the amount of output expected.
+ // Since this method is currently used only by unit tests,
+ // large-ish fixed buffer sizes have been chosen. If this
+ // method becomes used for something in production, it might
+ // be better to have the caller provide them as arguments.
+ if (appInput != null) {
+ OutputStream out =
+ new BufferedOutputStream(
+ p.getOutputStream(),
+ 100 * 1024);
+ int c;
+ while ((c = appInput.read()) != -1) {
+ out.write(c);
+ }
+ out.flush();
+ }
+ if (appOutput != null) {
+ InputStream in =
+ new BufferedInputStream(
+ p.getInputStream(),
+ 100 * 1024);
+ int c;
+ while ((c = in.read()) != -1) {
+ appOutput.write(c);
+ }
+ appOutput.flush();
+ in.close();
+ }
+ p.waitFor();
+
+ int status = p.exitValue();
+ if (logger != null) {
+ logger.info("exit status=" + status + " from " + pb.command());
+ }
+ return status;
+ }
+}