Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Make VAVr Java 8 & 9 compatible #2105

Merged
merged 5 commits into from
Sep 25, 2017
Merged

Make VAVr Java 8 & 9 compatible #2105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e357ac2
Fix Java 9 warnings
Sep 23, 2017
29c7ac5
Enable code generation on Java 9
Sep 23, 2017
f275dd9
Update the rest of the build dependencies
Sep 23, 2017
354965f
Enable the `vavr-benchmark` module
Sep 23, 2017
357ed2f
Add Java 9 as an alternative Travis build
Sep 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
language: java
jdk:
- oraclejdk8
- oraclejdk9
env:
global:
- secure: HU0QOLEDkYHcjX+PkicGGQ5g2L1/19V2fH13OSn82DOFlIiWmCfhSI8EG/QoWwPDp/HYQ4DH5Lo+ZEhVz5tgEultItyJ38LZJl/YYOzSAiRvTezmDdz+Cdg6SXAwCvgFCTagEJ1ChL5NyenDGDlwPB/drN0HlNNU6igPqNkM8l0=
Expand All @@ -9,7 +10,7 @@ env:
- secure: m91QxOLHBRnJzRyphCivL79i90pE/FO4yg8qTOaDuyTqVXBKnHmdARFOxSMZN9Qn3B3e28xxPaazIsJGCuPXiQKSObqzPLeFHUOa3atUJSckEUW623l1313A9iMFOYbqSBAHFHmiuuAJBwN/E2OnG3BcLZV1FRibW6J1qLRLdJw=
before_install:
- sudo apt-get update
- sudo apt-get install --only-upgrade -y oracle-java8-installer
- sudo apt-get install --only-upgrade --assume-yes oracle-java8-installer oracle-java9-installer
- pip install --user codecov
install: true
script: .travis/run-tests.sh
Expand Down
23 changes: 10 additions & 13 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ We use these goals frequently to keep the dependencies and plugins up-to-date:
<description>Vavr (formerly called Javaslang) is an object-functional language extension to Java 8+.</description>
<modules>
<module>vavr</module>
<module>vavr-benchmark</module>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does a Java 9 build require now this module? Just asking...
I ask myself if now a jar is released for vavr-benchmark, too (i.e. to maven central).
Is the benchmark profile then still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in a separate commit, because it's obviously not strictly related :)
It should be part of the project so that it is compile-time checked. I will disable the deployment, it's a valid concern.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked it, since it has tests only, there's nothing to deploy.
The benchmark profile has a run config also, so it still has its purpose - though we can remove the duplicate modules part, thanks for the note! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I had to test this module for Java 9 compatibility also, that's why I modified this part also)

Copy link
Contributor

@danieldietrich danieldietrich Sep 24, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benchmarks should not be part of the main build, even if it is a minimal benchmark.
If there are assertions/checks, they should be moved to the vavr core unit tests.
Benchmarks are about measuring. The only valid case to run them as part of the tests would be that we break the build if the performance decreases. But that is (currently) not possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The benchmarks aren't run during the build. Not even the assertions. just the compilation.
They are just part of the project, to make sure any refactoring is propagated there also.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that's cool.

<module>vavr-match</module>
<module>vavr-test</module>
</modules>
Expand Down Expand Up @@ -61,28 +62,28 @@ We use these goals frequently to keep the dependencies and plugins up-to-date:
<assertj.core.version>3.8.0</assertj.core.version>
<eclipse.lifecycle.mapping.version>1.0.0</eclipse.lifecycle.mapping.version>
<java.version>1.8</java.version>
<jmh.version>1.18</jmh.version>
<jmh.version>1.19</jmh.version>
<junit.version>4.12</junit.version>
<gwt.version>2.8.0</gwt.version>
<maven.enforcer.version>1.4.1</maven.enforcer.version>
<maven.build-helper.version>1.12</maven.build-helper.version>
<maven.bundle.version>3.2.0</maven.bundle.version>
<maven.build-helper.version>3.0.0</maven.build-helper.version>
<maven.bundle.version>3.3.0</maven.bundle.version>
<maven.clean.version>3.0.0</maven.clean.version>
<maven.install.version>2.5.2</maven.install.version>
<maven.compiler.version>3.6.0</maven.compiler.version>
<maven.compiler.version>3.7.0</maven.compiler.version>
<maven.deploy.version>2.8.2</maven.deploy.version>
<maven.gpg.version>1.6</maven.gpg.version>
<maven.jacoco.version>0.7.8</maven.jacoco.version>
<maven.jacoco.version>0.7.9</maven.jacoco.version>
<maven.jar.version>3.0.2</maven.jar.version>
<maven.javadoc.version>2.10.4</maven.javadoc.version>
<maven.release.version>2.5.3</maven.release.version>
<maven.versions.version>2.3</maven.versions.version>
<maven.versions.version>2.4</maven.versions.version>
<maven.surefire.version>2.19.1</maven.surefire.version>
<maven.source.version>3.0.1</maven.source.version>
<maven.exec.version>1.5.0</maven.exec.version>
<maven.gwt.plugin>1.0-rc-6</maven.gwt.plugin>
<scala.maven.version>3.3.1</scala.maven.version>
<scala.version>2.11.7</scala.version>
<scala.version>2.12.2</scala.version>
</properties>
<dependencyManagement>
<dependencies>
Expand Down Expand Up @@ -253,6 +254,8 @@ We use these goals frequently to keep the dependencies and plugins up-to-date:
<arg>-Xlint:all</arg>
<!-- Workaround for JDK bug https://bugs.openjdk.java.net/browse/JDK-6999068 -->
<arg>-Xlint:-processing</arg>
<!-- Enable Java 9 compilation with 1.8 compatibility -->
<arg>-Xlint:-options</arg>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do we use the options? Is it the implicit <target>${java.version}</target>?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is needed because java.version is always 8, even if the selected JDK is 9. The JDK cannot compile to another bytecode version, giving a warning. This disables the warning, since we're only checking the build, not producing any artifacts.

<java.version>1.8</java.version>

</compilerArgs>
</configuration>
</plugin>
Expand Down Expand Up @@ -456,12 +459,6 @@ We use these goals frequently to keep the dependencies and plugins up-to-date:
<!-- A profile for running the benchmarks -->
<profile>
<id>benchmark</id>
<modules>
<module>vavr</module>
<module>vavr-benchmark</module>
<module>vavr-match</module>
<module>vavr-test</module>
</modules>
<build>
<plugins>
<plugin>
Expand Down
1 change: 1 addition & 0 deletions vavr-benchmark/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
<includes>
<include>**/*Benchmark.java</include>
</includes>
<skip>true</skip>
</configuration>
</plugin>
<plugin>
Expand Down
3 changes: 2 additions & 1 deletion vavr/src/main/java/io/vavr/Lazy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ public static <T> Lazy<T> of(Supplier<? extends T> supplier) {
* @return A lazy sequence of values.
* @throws NullPointerException if values is null
*/
@SuppressWarnings("Convert2MethodRef") // TODO should be fixed in JDK 9 and Idea
public static <T> Lazy<Seq<T>> sequence(Iterable<? extends Lazy<? extends T>> values) {
Objects.requireNonNull(values, "values is null");
return Lazy.of(() -> Vector.ofAll(values).map(Lazy::get));
return Lazy.of(() -> Vector.ofAll(values).map(lazy -> lazy.get()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does a java 9 build not compile with the method reference?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's weird :/
We should probably open a bug for the JDK and for Idea

}

/**
Expand Down
4 changes: 2 additions & 2 deletions vavr/src/test/java/io/vavr/AssertionsExtensions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ public static class ClassAssert {
this.clazz = clazz;
}

@SuppressWarnings("deprecation")
public void isNotInstantiable() {
final Constructor<?> cons;
try {
cons = clazz.getDeclaredConstructor();
final Constructor<?> cons = clazz.getDeclaredConstructor();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

Assertions.assertThat(cons.isAccessible()).isFalse();
} catch (NoSuchMethodException e) {
throw new AssertionError("no default constructor found");
Expand Down
14 changes: 8 additions & 6 deletions vavr/src/test/java/io/vavr/MatchTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,23 @@
*/
package io.vavr;

import io.vavr.control.Either;
import io.vavr.match.annotation.Patterns;
import io.vavr.collection.List;
import io.vavr.control.Either;
import io.vavr.control.Option;
import io.vavr.control.Option.Some;
import io.vavr.match.annotation.Patterns;
import io.vavr.match.annotation.Unapply;
import org.junit.Test;

import java.math.BigDecimal;
import java.time.Year;
import java.util.function.BiFunction;
import java.util.function.Predicate;

import static io.vavr.API.$;
import static io.vavr.API.*;
import static io.vavr.Patterns.*;
import static io.vavr.MatchTest_DeveloperPatterns.$Developer;
import static io.vavr.Patterns.*;
import static io.vavr.Predicates.*;
import static org.assertj.core.api.Assertions.assertThat;

Expand Down Expand Up @@ -318,9 +320,9 @@ public void shouldDecomposeListOfTuple3() {
@SuppressWarnings("UnnecessaryLocalVariable")
@Test
public void shouldDecomposeListWithNonEmptyTail() {
final List<Option<Number>> intOptionList = List.of(Option.some(1), Option.some(2.0));
final String actual = Match(intOptionList).of(
Case($Cons($Some($(1)), $Cons($Some($(2.0)), $())), (x, xs) -> {
final List<Option<Number>> numberOptionList = List.of(Option.some(1), Option.some(2.0));
final String actual = Match(numberOptionList).of(
Case($Cons($Some($(1)), $Cons($Some($(2.0)), $())), (Some<Number> x, List<Option<Number>> xs) -> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danieldietrich, it only compiles on Java 9 if we explicitly specify the types here. Is that still ok?

// types are inferred correctly!
final Some<Number> head = x;
final List<Option<Number>> tail = xs;
Expand Down