yegor256 / cactoos

Object-Oriented Java primitives, as an alternative to Google Guava and Apache Commons
https://www.cactoos.org
MIT License
737 stars 163 forks source link

(#1569) Generify `org.cactoos.scalar` package #1626

Closed rocket-3 closed 2 years ago

rocket-3 commented 3 years ago

For #1569.

Relaxed parameter types by adding wildcards in constructors of classes of org.cactoos.scalar package:

victornoel commented 2 years ago

@0crat status

codecov-commenter commented 2 years ago

Codecov Report

Merging #1626 (38b0cee) into master (445cf16) will decrease coverage by 0.01%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1626      +/-   ##
============================================
- Coverage     90.12%   90.10%   -0.02%     
- Complexity     1602     1603       +1     
============================================
  Files           298      298              
  Lines          3745     3749       +4     
  Branches        122      122              
============================================
+ Hits           3375     3378       +3     
  Misses          337      337              
- Partials         33       34       +1     
Impacted Files Coverage Δ
src/main/java/org/cactoos/scalar/Retry.java 83.33% <ø> (ø)
...main/java/org/cactoos/scalar/ScalarOfSupplier.java 100.00% <ø> (ø)
src/main/java/org/cactoos/scalar/Ternary.java 100.00% <ø> (ø)
src/main/java/org/cactoos/scalar/Xor.java 100.00% <ø> (ø)
src/main/java/org/cactoos/scalar/And.java 100.00% <100.00%> (ø)
src/main/java/org/cactoos/scalar/AndInThreads.java 84.09% <100.00%> (ø)
src/main/java/org/cactoos/scalar/AndWithIndex.java 69.56% <100.00%> (ø)
...main/java/org/cactoos/scalar/CallableEnvelope.java 100.00% <100.00%> (ø)
...c/main/java/org/cactoos/scalar/EqualsNullable.java 85.71% <100.00%> (ø)
src/main/java/org/cactoos/scalar/FirstOf.java 91.66% <100.00%> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 445cf16...38b0cee. Read the comment docs.

victornoel commented 2 years ago

@0crat status

rocket-3 commented 2 years ago

@0crat status

rocket-3 commented 2 years ago

@rocket-3 just a few comments. Also, one last check: have you verified that the scalar package is completely covered with those relaxed generics?

@victornoel Yes, it's about all the package. If you're going to trust me, I've checked it again yesterday.

rocket-3 commented 2 years ago

@0crat status

victornoel commented 2 years ago

@rocket-3 I do trust you, my question is just a "protocol" check just to ensure you checked it, not to doubt that you did ^^ that's why we have reviews :)

victornoel commented 2 years ago

@rocket-3 so what about #1628 and the comment I made there: https://github.com/yegor256/cactoos/pull/1628#issuecomment-927296249 ? If the present PR is the one you want to keep, then please close the other

rocket-3 commented 2 years ago

@victornoel It (#1628) was just the same, I've closed it.

victornoel commented 2 years ago

@rocket-3 ok, cool, thx!

victornoel commented 2 years ago

@rultor merge

rultor commented 2 years ago

@rultor merge

@victornoel OK, I'll try to merge now. You can check the progress of the merge here

rultor commented 2 years ago

@rultor merge

@rocket-3 @victornoel Oops, I failed. You can see the full log here (spent 4min)

[\u001b[1;31mERROR\u001b[m] \u001b[1;31m\u001b[m
[\u001b[1;31mERROR\u001b[m] -> \u001b[1m[Help 1]\u001b[m
\u001b[1;31morg.apache.maven.lifecycle.LifecycleExecutionException\u001b[m: \u001b[1;31mFailed to execute goal \u001b[32morg.apache.maven.plugins:maven-compiler-plugin:3.8.1:testCompile\u001b[m \u001b[1m(default-testCompile)\u001b[m on project \u001b[36mcactoos\u001b[m: \u001b[1;31mCompilation failure
/home/r/repo/src/test/java/org/cactoos/scalar/AndTest.java: warnings found and -Werror specified
\u001b[m\u001b[m
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.MojoExecutor.execute (\u001b[1mMojoExecutor.java:215\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.MojoExecutor.execute (\u001b[1mMojoExecutor.java:156\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.MojoExecutor.execute (\u001b[1mMojoExecutor.java:148\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (\u001b[1mLifecycleModuleBuilder.java:117\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (\u001b[1mLifecycleModuleBuilder.java:81\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (\u001b[1mSingleThreadedBuilder.java:56\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.LifecycleStarter.execute (\u001b[1mLifecycleStarter.java:128\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.DefaultMaven.doExecute (\u001b[1mDefaultMaven.java:305\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.DefaultMaven.doExecute (\u001b[1mDefaultMaven.java:192\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.DefaultMaven.execute (\u001b[1mDefaultMaven.java:105\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.cli.MavenCli.execute (\u001b[1mMavenCli.java:956\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.cli.MavenCli.doMain (\u001b[1mMavenCli.java:288\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.cli.MavenCli.main (\u001b[1mMavenCli.java:192\u001b[m)
    \u001b[1mat\u001b[m sun.reflect.NativeMethodAccessorImpl.invoke0 (\u001b[1mNative Method\u001b[m)
    \u001b[1mat\u001b[m sun.reflect.NativeMethodAccessorImpl.invoke (\u001b[1mNativeMethodAccessorImpl.java:62\u001b[m)
    \u001b[1mat\u001b[m sun.reflect.DelegatingMethodAccessorImpl.invoke (\u001b[1mDelegatingMethodAccessorImpl.java:43\u001b[m)
    \u001b[1mat\u001b[m java.lang.reflect.Method.invoke (\u001b[1mMethod.java:498\u001b[m)
    \u001b[1mat\u001b[m org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (\u001b[1mLauncher.java:282\u001b[m)
    \u001b[1mat\u001b[m org.codehaus.plexus.classworlds.launcher.Launcher.launch (\u001b[1mLauncher.java:225\u001b[m)
    \u001b[1mat\u001b[m org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (\u001b[1mLauncher.java:406\u001b[m)
    \u001b[1mat\u001b[m org.codehaus.plexus.classworlds.launcher.Launcher.main (\u001b[1mLauncher.java:347\u001b[m)
\u001b[1mCaused by\u001b[m: org.apache.maven.plugin.compiler.CompilationFailureException: \u001b[1;31mCompilation failure
/home/r/repo/src/test/java/org/cactoos/scalar/AndTest.java: warnings found and -Werror specified
\u001b[m
    \u001b[1mat\u001b[m org.apache.maven.plugin.compiler.AbstractCompilerMojo.execute (\u001b[1mAbstractCompilerMojo.java:1220\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.plugin.compiler.TestCompilerMojo.execute (\u001b[1mTestCompilerMojo.java:180\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (\u001b[1mDefaultBuildPluginManager.java:137\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.MojoExecutor.execute (\u001b[1mMojoExecutor.java:210\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.MojoExecutor.execute (\u001b[1mMojoExecutor.java:156\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.MojoExecutor.execute (\u001b[1mMojoExecutor.java:148\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (\u001b[1mLifecycleModuleBuilder.java:117\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (\u001b[1mLifecycleModuleBuilder.java:81\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (\u001b[1mSingleThreadedBuilder.java:56\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.lifecycle.internal.LifecycleStarter.execute (\u001b[1mLifecycleStarter.java:128\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.DefaultMaven.doExecute (\u001b[1mDefaultMaven.java:305\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.DefaultMaven.doExecute (\u001b[1mDefaultMaven.java:192\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.DefaultMaven.execute (\u001b[1mDefaultMaven.java:105\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.cli.MavenCli.execute (\u001b[1mMavenCli.java:956\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.cli.MavenCli.doMain (\u001b[1mMavenCli.java:288\u001b[m)
    \u001b[1mat\u001b[m org.apache.maven.cli.MavenCli.main (\u001b[1mMavenCli.java:192\u001b[m)
    \u001b[1mat\u001b[m sun.reflect.NativeMethodAccessorImpl.invoke0 (\u001b[1mNative Method\u001b[m)
    \u001b[1mat\u001b[m sun.reflect.NativeMethodAccessorImpl.invoke (\u001b[1mNativeMethodAccessorImpl.java:62\u001b[m)
    \u001b[1mat\u001b[m sun.reflect.DelegatingMethodAccessorImpl.invoke (\u001b[1mDelegatingMethodAccessorImpl.java:43\u001b[m)
    \u001b[1mat\u001b[m java.lang.reflect.Method.invoke (\u001b[1mMethod.java:498\u001b[m)
    \u001b[1mat\u001b[m org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (\u001b[1mLauncher.java:282\u001b[m)
    \u001b[1mat\u001b[m org.codehaus.plexus.classworlds.launcher.Launcher.launch (\u001b[1mLauncher.java:225\u001b[m)
    \u001b[1mat\u001b[m org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (\u001b[1mLauncher.java:406\u001b[m)
    \u001b[1mat\u001b[m org.codehaus.plexus.classworlds.launcher.Launcher.main (\u001b[1mLauncher.java:347\u001b[m)
[\u001b[1;31mERROR\u001b[m] 
[\u001b[1;31mERROR\u001b[m] Re-run Maven using the \u001b[1m-X\u001b[m switch to enable full debug logging.
[\u001b[1;31mERROR\u001b[m] 
[\u001b[1;31mERROR\u001b[m] For more information about the errors and possible solutions, please read the following articles:
[\u001b[1;31mERROR\u001b[m] \u001b[1m[Help 1]\u001b[m http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
container b326520ee6e8921a150b7a233abb29c8c8d6d946c1873a9413400cc030d63e7e is dead
Fri Oct  8 16:15:01 CEST 2021
rocket-3 commented 2 years ago

@victornoel, can't reproduce, cause I don't have ../settings.xml and the warnings doesn't appear without it.

@rultor merge

@rocket-3 @victornoel Oops, I failed. You can see the full log here (spent 4min)


+ mvn clean install -Pqulice --errors --settings ../settings.xml

...

[1;33mWARNINGm] COMPILATION WARNING : [1;34mINFOm] ------------------------------------------------------------- [1;33mWARNINGm] /home/r/repo/src/test/java/org/cactoos/scalar/AndTest.java:[97,17] unchecked generic array creation for varargs parameter of type org.cactoos.Func<? super java.lang.Object,java.lang.Boolean>[] [1;33mWARNINGm] /home/r/repo/src/test/java/org/cactoos/scalar/AndTest.java:[108,17] unchecked generic array creation for varargs parameter of type org.cactoos.Func<? super java.lang.Object,java.lang.Boolean>[] [1;33mWARNINGm] /home/r/repo/src/test/java/org/cactoos/scalar/ItemAtTest.java:[60,30] unchecked generic array creation for varargs parameter of type X[] [1;33mWARNINGm] /home/r/repo/src/test/java/org/cactoos/scalar/ItemAtTest.java:[60,30] unchecked generic array creation for varargs parameter of type X[] [1;33mWARNINGm] /home/r/repo/src/test/java/org/cactoos/scalar/ItemAtTest.java:[72,23] unchecked generic array creation for varargs parameter of type X[] [1;33mWARNINGm] /home/r/repo/src/test/java/org/cactoos/scalar/ItemAtTest.java:[72,23] unchecked generic array creation for varargs parameter of type X[]

victornoel commented 2 years ago

@rocket-3 even when running mvn manually with Java 8? I don't believe it's related to the settings.xml file (which is only there for credentials used by rultor). This is a typical problem with varargs and generics in Java, just add suppress warnings for those tests, as in here: https://github.com/yegor256/cactoos/blob/9f33693c15e48e10e7e5a62bbb32c66f26200c62/src/test/java/org/cactoos/set/SetOfTest.java#L46

I'm a bit surprised that the CI didn't complain though... but well, let's go with it and add that fix to the corresponding tests :)

rocket-3 commented 2 years ago

@victornoel Please, try to merge now

victornoel commented 2 years ago

@rultor merge

rultor commented 2 years ago

@rultor merge

@victornoel OK, I'll try to merge now. You can check the progress of the merge here

rultor commented 2 years ago

@rultor merge

@victornoel Done! FYI, the full log is here (took me 11min)

victornoel commented 2 years ago

@rocket-3 👍

rocket-3 commented 2 years ago

@0crat, status

0crat commented 2 years ago

Are you speaking to me or about me here; you must always start your message with my name if you want to address it to me, see §1

victornoel commented 2 years ago

@0crat status