t0mmyw
t0mmyw

Reputation: 737

Sonar Change this condition so that it does not always evaluate to "true". False positive

Our sonar is giving a Change this condition so that it does not always evaluate to "true" issue and I think it is a false positive

    for (Path file: stream) {
        FileDetails fileDetails = getFileDetails(path.toString() + '/' + file.getFileName().toString(), file);
        if (fileDetails != null) {
            fileList.add(fileDetails);
        }
    }
    // Process the list of non null files
    ...

The get file details method as defined later in class

private FileDetails getFileDetails(String absolutePathName, Path path) {
    FileDetails fileDetails = new FileDetails();
    fileDetails.setName(path.getFileName().toString());
    fileDetails.setAbsoluteName(absolutePathName);
    fileDetails.setDirectory(path.toFile().isDirectory());
    fileDetails.setFileStoreUri(fileStoreDetails.getUri());
    try {
        fileDetails.setSize(Files.size(path));
        fileDetails.setModifiedDate(new Date(Files.getLastModifiedTime(path).toMillis()));
    } catch (java.nio.file.AccessDeniedException e) {
        logger.info("Cannot get file details for " + path, e);
        fileDetails = null;
    } catch (IOException e) {
        // No need to throw a checked exception as we can't do anything with it.
        logger.error("Cannot get file details for " + path, e);
        throw new UncheckedIOException(e);
    }
    return fileDetails;
}

From my reading, fileDetails will most likely be non-null, but in a certain scenario it will be null, hence the check. Is there a trick here that I'm missing?

We're using SonarQube 6.2, with the v4.4.0.8066 of the java plugin

Upvotes: 0

Views: 3024

Answers (1)

t0mmyw
t0mmyw

Reputation: 737

So, java.nio.file.AccessDeniedException is a sub class of IOException and isn't directly thrown. After review of the function, we decided that returning null for access denied is the wrong thing to do. So we removed this catch (leaving the IOException catch in place). This meant the method now can't return null. Which made the if redundant as Sonar tells us

Upvotes: 2

Related Questions