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

GH-30: Handle files that are empty, have an unknown newline, or can't… #33

Merged
merged 1 commit into from
Sep 9, 2020
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
50 changes: 50 additions & 0 deletions src/main/java/net/revelc/code/impsort/EmptyFileException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package net.revelc.code.impsort;

import java.io.IOException;
import java.nio.file.Path;

/**
* Signals that the file denoted by this path is empty.
*
* <p>
* This exception will be thrown by the {@link ImpSort#parseFile} when it encounters an empty file.
*/
public class EmptyFileException extends IOException {
private static final long serialVersionUID = 1L;

private final Path path;

/**
* Constructs a {@code EmptyFileException} with the specified path.
*
* @param path the path
*/
public EmptyFileException(final Path path) {
super("Empty file " + path);
this.path = path;
}

/**
* Returns the path.
*
* @return the path
*/
public Path getPath() {
return path;
}

}
30 changes: 25 additions & 5 deletions src/main/java/net/revelc/code/impsort/ImpSort.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import com.github.javaparser.ast.Node;
import com.github.javaparser.ast.NodeList;
import com.github.javaparser.ast.PackageDeclaration;
import com.github.javaparser.ast.body.TypeDeclaration;
import com.github.javaparser.ast.comments.Comment;
import com.github.javaparser.ast.comments.JavadocComment;
import com.github.javaparser.javadoc.Javadoc;
Expand Down Expand Up @@ -73,9 +72,27 @@ public ImpSort(final Charset sourceEncoding, final Grouper grouper, final boolea
this.lineEnding = lineEnding;
}

public Result parseFile(final Path path) throws IOException {
String file = new String(Files.readAllBytes(path), sourceEncoding);
/**
* Parses the file denoted by this path and returns the result.
*
* @param path the path
* @return the result
* @throws IOException if the Java file denoted by this path can't be parsed
* @throws EmptyFileException if the Java file denoted by this path is empty (zero bytes)
* @throws UnknownLineEndingException if the Java file denoted by this path has an unknown line
* ending
*/
public Result parseFile(final Path path)
throws IOException, EmptyFileException, UnknownLineEndingException {
byte[] buf = Files.readAllBytes(path);
if (buf.length == 0) {
throw new EmptyFileException(path);
}
String file = new String(buf, sourceEncoding);
LineEnding fileLineEnding = LineEnding.determineLineEnding(file);
if (fileLineEnding == LineEnding.UNKNOWN) {
throw new UnknownLineEndingException(path);
}
LineEnding impLineEnding;
if (lineEnding == LineEnding.KEEP) {
impLineEnding = fileLineEnding;
Expand All @@ -84,8 +101,11 @@ public Result parseFile(final Path path) throws IOException {
}
List<String> fileLines = Arrays.asList(file.split(fileLineEnding.getChars()));
ParseResult<CompilationUnit> parseResult = new JavaParser().parse(file);
CompilationUnit unit =
parseResult.getResult().orElseThrow(() -> new IOException("Unable to parse " + path));
Optional<CompilationUnit> unitOptional = parseResult.getResult();
if (!parseResult.isSuccessful() || !unitOptional.isPresent()) {
throw new IOException("Unable to parse " + path);
}
CompilationUnit unit = unitOptional.get();
Comment on lines +104 to +108
Copy link
Member

Choose a reason for hiding this comment

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

I'd have to check the parser code, but I'm pretty sure the additional check to see if the parse is successful is redundant. It was successful already if the Optional contained a value. Pretty sure this can go back to what it was.

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 have a test case in the test file I created. The test case checks if an IOException is thrown.

Without this check, there is no error or anything (maybe it just leaves the file as-is).

The Optional has a value but the content seems to be set to something like ??? instead of Java code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So you might expect in the case of a syntax error that the Optional is not present, but that does not seem to be the case.

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 looked at the docs and it says Returns: the AST of the parsed source code, or empty if parsing failed completely. So, if it can parse some, but not completely, you will get a partial Result and not an empty Optional.

Position packagePosition =
unit.getPackageDeclaration().map(p -> p.getEnd().get()).orElse(unit.getBegin().get());
NodeList<ImportDeclaration> importDeclarations = unit.getImports();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package net.revelc.code.impsort;

import java.io.IOException;
import java.nio.file.Path;

/**
* Signals that the file denoted by this path has an unknown line ending.
*
* <p>
* This exception will be thrown by the {@link ImpSort#parseFile} when it encounters a file with an
* unknown line ending.
*/
public class UnknownLineEndingException extends IOException {
private static final long serialVersionUID = 1L;

private final Path path;

/**
* Constructs a {@code UnknownLineEndingException} with the specified path.
*
* @param path the path
*/
public UnknownLineEndingException(final Path path) {
super("Unknown line ending for file " + path);
this.path = path;
}

/**
* Returns the path.
*
* @return the path
*/
public Path getPath() {
return path;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@
import org.apache.maven.project.MavenProject;
import org.codehaus.plexus.util.DirectoryScanner;

import net.revelc.code.impsort.EmptyFileException;
import net.revelc.code.impsort.Grouper;
import net.revelc.code.impsort.ImpSort;
import net.revelc.code.impsort.LineEnding;
import net.revelc.code.impsort.Result;
import net.revelc.code.impsort.UnknownLineEndingException;

abstract class AbstractImpSortMojo extends AbstractMojo {

Expand Down Expand Up @@ -233,6 +235,10 @@ public final void execute() throws MojoExecutionException, MojoFailureException
numProcessed.getAndIncrement();
}
processResult(path, result);
} catch (EmptyFileException e) {
getLog().warn("Skipping empty file " + e.getPath());
} catch (UnknownLineEndingException e) {
getLog().warn("Skipping file with unknown line ending " + e.getPath());
} catch (IOException e) {
fail("Error reading file " + path, e);
}
Expand Down
82 changes: 82 additions & 0 deletions src/test/java/net/revelc/code/impsort/LineEndingEdgeCasesTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package net.revelc.code.impsort;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

/**
* Test class for special file cases.
*/
public class LineEndingEdgeCasesTest {

private static Grouper eclipseDefaults =
new Grouper("java.,javax.,org.,com.", "", false, false, true);

@Rule
public TemporaryFolder folder = new TemporaryFolder();

/**
* Test successfully parsing empty file (zero bytes).
*/
@Test
public void testEmptyFile() throws IOException {
Path p = folder.newFile("EmptyFile.java").toPath();
Files.write(p, new byte[0]);
EmptyFileException e = assertThrows(EmptyFileException.class,
() -> new ImpSort(StandardCharsets.UTF_8, eclipseDefaults, true, true, LineEnding.AUTO)
.parseFile(p));
assertEquals("Empty file " + p, e.getMessage());
}

/**
* Test successfully parsing file without any line ending.
*/
@Test
public void testFileWithoutLineEnding() throws IOException {
String s =
"import java.lang.System;public class FileWithoutNewline{public static void main(String[] args){System.out.println(\"Hello, world!\");}}";
Path p = folder.newFile("FileWithoutLineEnding.java").toPath();
Files.write(p, s.getBytes(StandardCharsets.UTF_8));
UnknownLineEndingException e = assertThrows(UnknownLineEndingException.class,
() -> new ImpSort(StandardCharsets.UTF_8, eclipseDefaults, true, true, LineEnding.AUTO)
.parseFile(p));
assertEquals("Unknown line ending for file " + p, e.getMessage());
}

/**
* Test successfully parsing file that can't be parsed.
*/
@Test
public void testInvalidFile() throws IOException {
String s = "public class InvalidFile {\n" + " public static void main(String[] args) {\n"
+ " System.out.println(\"Hello, world!\")\n" + " }\n" + "}\n";
Path p = folder.newFile("InvalidFile.java").toPath();
Files.write(p, s.getBytes(StandardCharsets.UTF_8));
IOException e = assertThrows(IOException.class,
() -> new ImpSort(StandardCharsets.UTF_8, eclipseDefaults, true, true, LineEnding.AUTO)
.parseFile(p));
assertEquals("Unable to parse " + p, e.getMessage());
}

}