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.

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

Multiple targets in lint runner #40

Merged
merged 1 commit into from
Jun 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ test {
}

dependencies {
implementation group: 'commons-io', name: 'commons-io', version: '2.6'
ankuagarwal marked this conversation as resolved.
Show resolved Hide resolved
implementation group: 'org.json', name: 'json', version: '20180813'
implementation group: 'com.j2html', name: 'j2html', version: '1.4.0'

Expand Down
82 changes: 40 additions & 42 deletions src/main/java/com/zachary_moore/runner/LintRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,78 +5,74 @@
import com.zachary_moore.lint.LintRegister;
import com.zachary_moore.lint.LintRule;
import com.zachary_moore.objects.JSONFile;

import java.io.File;
import java.io.IOException;
import java.util.*;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import org.apache.commons.io.FileUtils;


public class LintRunner {

private List<JSONFile> filesToLint;
private String[] basePaths;
private Map<LintRule, Map<JSONFile, List<String>>> lintOutput;
private final LintRegister lintRegister;

/**
* Create a LintRunner
* @param lintRegister representation of {@link LintRule} to run against
* @param basePath Base Path to setup {@link JSONFile} from
* @param basePaths Paths to folders or files to fetch {@link JSONFile}(s) from
*/
public LintRunner(LintRegister lintRegister,
String basePath) {
String... basePaths) {
this.lintRegister = lintRegister;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved most of this logic to getAllFiles. Failing to create an instance because one of the paths is wrong is not very good. Also constructors should not have complicated logic.

File basePathFile = new File(basePath);
if (basePathFile.isFile()) {
try {
this.filesToLint = new ArrayList<>(Collections.
singletonList(new JSONFile(basePathFile)));
} catch (IOException e) {
throw new RuntimeException(e);
}
} else if (basePathFile.isDirectory()){
this.filesToLint = getAllFiles(basePath).stream()
.map(f -> {
try {
return new JSONFile(f);
} catch (IOException e) {
throw new RuntimeException(e);
}
})
.filter(Objects::nonNull)
.collect(Collectors.toList());
} else {
throw new RuntimeException("Input path is neither a file nor directory");
}
this.basePaths = basePaths;
}

private List<File> getAllFiles(String basePath) {
List<File> files = new ArrayList<>();
File directory = new File(basePath);
private Set<JSONFile> getFilesToLint() {
Set<File> files = new HashSet<>();

File[] fList = directory.listFiles();
if (fList != null) {
for (File file : fList) {
if (file.isFile()) {
files.add(file);
} else if (file.isDirectory()) {
files.addAll(getAllFiles(file.getAbsolutePath()));
}
for (String basePath : this.basePaths) {
File file = new File(basePath);

if (file.isFile()) {
files.add(file);
} else if (file.isDirectory()) {
files.addAll(FileUtils.listFiles(file, null, true));
} else {
// TODO: Add proper logging
Logger.getGlobal().warning(basePath + " is not a valid file path");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would love feedback here. We don't want to fail the runner because one of the provided paths was wrong. But it should be more conspicuous than a log. Is it possible to include this failure as a part of the report?

}
}
return files;

return files.stream().map(file -> {
try {
return new JSONFile(file);
} catch (IOException e) {
// Is not a JSON file
return null;
}
}).filter(Objects::nonNull).collect(Collectors.toSet());
}

/**
* Lint all files in given path in constructor
* Lint all files in given path in constructor and store the output
* @return Representation of any lint issues
*/
public Map<LintRule, Map<JSONFile, List<String>>> lint() {
Map<LintRule, Map<JSONFile, List<String>>> lintOutput = new HashMap<>();
Set<JSONFile> filesToLint = getFilesToLint();

for (LintRule lintRule : lintRegister.getLintRules()) {
try {
if (lintRule.getLevel() != LintLevel.IGNORE) {
Map<JSONFile, List<String>> lintReports = lintRule.lint(filesToLint.toArray(new JSONFile[filesToLint.size()]));
Map<JSONFile, List<String>> lintReports = lintRule.lint(filesToLint.toArray(new JSONFile[0]));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Better for performance. No change in semantics.

PawanHegde marked this conversation as resolved.
Show resolved Hide resolved
if (lintReports.size() != 0) {
lintOutput.put(lintRule, lintReports);
}
Expand All @@ -87,6 +83,7 @@ public Map<LintRule, Map<JSONFile, List<String>>> lint() {
}
}
this.lintOutput = lintOutput;

return lintOutput;
}

Expand All @@ -105,6 +102,7 @@ public int analyzeLintAndGiveExitCode() {
return 1;
}
}

return 0;
}
}
15 changes: 15 additions & 0 deletions src/test/java/com/zachary_moore/runner/LintRunnerShould.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,19 @@ public void lintRunnerShouldGiveExitStatus0WithIgnore() throws Exception {
lintRunner.lint();
assert(lintRunner.analyzeLintAndGiveExitCode() == 0);
}

@Test
public void lintRunnerShouldTakeMultipleFiles() throws Exception {
LintRule lintRule = builder.setLevel(LintLevel.ERROR).build();
this.lintRegister.register(lintRule);
LintRunner lintRunner =
new LintRunner(
this.lintRegister,
"./src/test/resources/test-2.json",
"./src/test/resources/test-file.pdsc");
Map<LintRule, Map<JSONFile, List<String>>> lintOutput = lintRunner.lint();

assert(lintRunner.analyzeLintAndGiveExitCode() == 1);
assert(lintOutput.get(lintRule).size() == 2);
}
}