Fix ErrorProne errors #3623

koppor · 2018-01-09T15:47:26Z

This fixes

/home/ubuntu/jabref/src/main/java/org/jabref/logic/citationstyle/CitationStyle.java:156: error: [StreamResourceLeak] Streams that encapsulate a closeable resource should be closed using try-with-resources
                List<Path> allStyles = Files.find(jarFs.getRootDirectories().iterator().next(), 1, (file, attr) -> file.toString().endsWith("csl")).collect(Collectors.toList());
                                                 ^
    (see http://errorprone.info/bugpattern/StreamResourceLeak)
  Did you mean 'List<Path> allStyles ;'?
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
1 error

Siedlerchr · 2018-01-09T15:57:05Z

src/main/java/org/jabref/logic/citationstyle/CitationStyle.java

-                List<Path> allStyles = Files.find(jarFs.getRootDirectories().iterator().next(), 1, (file, attr) -> file.toString().endsWith("csl")).collect(Collectors.toList());
-
+                List<Path> allStyles;
+                try (Stream<Path> stylefileStream = Files.find(jarFs.getRootDirectories().iterator().next(), 1, (file, attr) -> file.toString().endsWith("csl"))) {


This is again one of the ugly cases where an unchecked exception could be thrown. Because streams and lambdas can't throw checked exceptions. In this case it could be that the stream is already closed when calling collect.
Similar to #3606

Yes, please catch IOUncheckedException and rethrow it as IOException.

tobiasdiez

Except the stuff concerning unchecked IO exceptions, this looks good.

tobiasdiez · 2018-01-09T16:36:29Z

src/main/java/org/jabref/logic/citationstyle/CitationStyle.java

-                List<Path> allStyles = Files.find(jarFs.getRootDirectories().iterator().next(), 1, (file, attr) -> file.toString().endsWith("csl")).collect(Collectors.toList());
-
+                List<Path> allStyles;
+                try (Stream<Path> stylefileStream = Files.find(jarFs.getRootDirectories().iterator().next(), 1, (file, attr) -> file.toString().endsWith("csl"))) {


Yes, please catch IOUncheckedException and rethrow it as IOException.

tobiasdiez · 2018-01-09T16:38:30Z

src/main/java/org/jabref/logic/util/io/FileUtil.java

@@ -331,8 +331,8 @@ public static String createDirNameFromPattern(BibDatabase database, BibEntry ent
     * @return the path to the first file that matches the defined conditions
     */
    public static Optional<Path> find(String filename, Path rootDirectory) {
-        try {
-            return Files.walk(rootDirectory)
+        try (Stream<Path> pathStream = Files.walk(rootDirectory)) {


Can you please check if walk may also throw an IOUncheckedEx and if yes, add it to the catch block.

Checked the JavaDoc and the sources... does not seem so.

EDIT: it does :/

tobiasdiez · 2018-01-09T16:40:19Z

src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java

-                            res.addAll(findFile(entry, subElement, restOfFileString, extensionRegExp));
-                        }
-                    });
+                try (Stream<Path> pathStream = Files.walk(actualDirectory)) {


tobiasdiez · 2018-01-09T16:40:47Z

src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java

+    }
+
+    private List<Path> collectMatchingFiles(Path actualDirectory, BiPredicate<Path, BasicFileAttributes> matcher) {
+        try (Stream<Path> pathStream = Files.find(actualDirectory, 1, matcher)) {


This reverts commit e12311f.

LinusDietz · 2018-01-10T09:23:33Z

@tobiasdiez @Siedlerchr Can you have a look if the exception handling is correct now? see c93ae63 for that

tobiasdiez · 2018-01-10T09:39:46Z

src/main/java/org/jabref/logic/util/io/RegExpBasedFileFinder.java

+        try (Stream<Path> pathStream = Files.find(actualDirectory, 1, matcher)) {
+            return pathStream.collect(Collectors.toList());
+        } catch (UncheckedIOException | IOException ioe) {
+            return Collections.emptyList();


I think this method should just rethrow as IOException.

tobiasdiez · 2018-01-10T09:42:20Z

@LinusDietz Thanks, the code looks good to me!

LinusDietz and others added 4 commits January 8, 2018 15:56

Explicitly set jacoco version

1367e19

Merge remote-tracking branch 'origin/master'

8a5578d

This fixes the error prone errors

7c8edce

Merge remote-tracking branch 'origin/master'

bb9406f

Siedlerchr reviewed Jan 9, 2018

View reviewed changes

LinusDietz added 3 commits January 9, 2018 17:22

Fix RegExpBasedFileFinder

915c7ec

Fix FileUtil

b48a6db

Fix LocalizationParser

b396517

tobiasdiez mentioned this pull request Jan 9, 2018

Catch and propagate UncheckedIOException in FileFinder #3603

Merged

6 tasks

LinusDietz added 4 commits January 9, 2018 17:35

Fix PropertiesLocaleCompletenessTest

1ca53a0

Fix ArchitectureTests

fd7b04e

Fix TestIconsProperties

81b5511

Merge branch 'master' into buildfix

9dcfee0

tobiasdiez requested changes Jan 9, 2018

View reviewed changes

LinusDietz added 7 commits January 9, 2018 17:54

Merge branch 'master' of https://github.com/JabRef/jabref into buildfix

5dc2bc3

Fix CiteKeyBasedFileFinder

cc4df3b

Addition to Merge

e12311f

Revert "Addition to Merge"

00691b6

This reverts commit e12311f.

Don't return empty lists

63d03e5

Fix bug

c8dce57

Test are green

14d07d2

LinusDietz changed the title ~~This fixes the error prone errors~~ Fix ErrorProne errors Jan 9, 2018

Checkstyle is green

f005832

koppor mentioned this pull request Jan 10, 2018

Add installation directory to Java version error message #3624

Merged

6 tasks

Catch UncheckedIOExceptions

c93ae63

LinusDietz added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 10, 2018

tobiasdiez reviewed Jan 10, 2018

View reviewed changes

Siedlerchr approved these changes Jan 10, 2018

View reviewed changes

Siedlerchr merged commit d0f6270 into master Jan 10, 2018

Siedlerchr deleted the buildfix branch January 10, 2018 09:53

koppor mentioned this pull request Jan 24, 2018

Problem when building from source for v4.1 #3657

Closed

1 task

k3KAW8Pnf7mkmdSMPHz27 mentioned this pull request Jan 14, 2021

Fix expansion of bracketed expressions in RegExpBasedFileFinder #7338

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ErrorProne errors #3623

Fix ErrorProne errors #3623

koppor commented Jan 9, 2018

Siedlerchr Jan 9, 2018

tobiasdiez Jan 9, 2018

tobiasdiez left a comment

tobiasdiez Jan 9, 2018

tobiasdiez Jan 9, 2018

LinusDietz Jan 10, 2018 •

edited

Loading

tobiasdiez Jan 9, 2018

tobiasdiez Jan 9, 2018

LinusDietz commented Jan 10, 2018

tobiasdiez Jan 10, 2018

tobiasdiez commented Jan 10, 2018

Fix ErrorProne errors #3623

Fix ErrorProne errors #3623

Conversation

koppor commented Jan 9, 2018

Siedlerchr Jan 9, 2018

Choose a reason for hiding this comment

tobiasdiez Jan 9, 2018

Choose a reason for hiding this comment

tobiasdiez left a comment

Choose a reason for hiding this comment

tobiasdiez Jan 9, 2018

Choose a reason for hiding this comment

tobiasdiez Jan 9, 2018

Choose a reason for hiding this comment

LinusDietz Jan 10, 2018 • edited Loading

Choose a reason for hiding this comment

tobiasdiez Jan 9, 2018

Choose a reason for hiding this comment

tobiasdiez Jan 9, 2018

Choose a reason for hiding this comment

LinusDietz commented Jan 10, 2018

tobiasdiez Jan 10, 2018

Choose a reason for hiding this comment

tobiasdiez commented Jan 10, 2018

LinusDietz Jan 10, 2018 •

edited

Loading