Skip to content

Commit

Permalink
Do less path string manipulation
Browse files Browse the repository at this point in the history
to avoid windows-specific issues with patch application.

RELNOTES: N/A

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=156451545
  • Loading branch information
cushon authored and ronshapiro committed May 24, 2017
1 parent 487d0ef commit 8a87d57
Show file tree
Hide file tree
Showing 13 changed files with 115 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ static RefactoringCollection refactor(PatchingOptions patchingOptions) {
Function<URI, RefactoringResult> postProcess;

if (patchingOptions.inPlace()) {
fileDestination = new FsFileDestination(rootPath);
fileDestination = new FsFileDestination();
postProcess =
uri ->
RefactoringResult.create(
Expand Down Expand Up @@ -160,7 +160,7 @@ RefactoringResult applyChanges(URI uri) throws Exception {
return RefactoringResult.create("", RefactoringResultType.NO_CHANGES);
}

doApplyProcess(fileDestination, new FsFileSource(rootPath), listeners);
doApplyProcess(fileDestination, new FsFileSource(), listeners);
return postProcess.apply(uri);
}

Expand All @@ -186,19 +186,16 @@ private void doApplyProcess(
Collection<DelegatingDescriptionListener> listeners) {
for (DelegatingDescriptionListener listener : listeners) {
try {
SourceFile file = fileSource.readFile(listener.base.getRelevantFileName());
SourceFile file = fileSource.readFile(listener.base.getPath());
listener.base.applyDifferences(file);
fileDestination.writeFile(file);
} catch (IOException e) {
logger.log(
Level.WARNING,
"Failed to apply diff to file " + listener.base.getRelevantFileName(),
e);
logger.log(Level.WARNING, "Failed to apply diff to file " + listener.base.getPath(), e);
}
}
}

private final class DelegatingDescriptionListener implements DescriptionListener {
private static final class DelegatingDescriptionListener implements DescriptionListener {
final DescriptionBasedDiff base;
final DescriptionListener listener;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import com.google.errorprone.matchers.Description;
import com.sun.tools.javac.tree.EndPosTable;
import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.LinkedHashSet;
import java.util.Set;

Expand All @@ -38,7 +40,7 @@
*/
public final class DescriptionBasedDiff implements DescriptionListener, Diff {

private final String sourcePath;
private final Path sourcePath;
private final boolean ignoreOverlappingFixes;
private final JCCompilationUnit compilationUnit;
private final Set<String> importsToAdd;
Expand All @@ -62,7 +64,7 @@ private DescriptionBasedDiff(
boolean ignoreOverlappingFixes,
ImportOrganizer importOrganizer) {
this.compilationUnit = checkNotNull(compilationUnit);
this.sourcePath = compilationUnit.getSourceFile().toUri().getPath();
this.sourcePath = Paths.get(compilationUnit.getSourceFile().toUri());
this.ignoreOverlappingFixes = ignoreOverlappingFixes;
this.importsToAdd = new LinkedHashSet<>();
this.importsToRemove = new LinkedHashSet<>();
Expand All @@ -71,7 +73,7 @@ private DescriptionBasedDiff(
}

@Override
public String getRelevantFileName() {
public Path getPath() {
return sourcePath;
}

Expand Down
6 changes: 4 additions & 2 deletions check_api/src/main/java/com/google/errorprone/apply/Diff.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,16 @@

package com.google.errorprone.apply;

import java.nio.file.Path;

/**
* All the differences to be applied to a source file to be applied in a refactoring.
*
* @author [email protected] (Simon Nickerson)
*/
public interface Diff {
/** Gets the name of the file this difference applies to */
public String getRelevantFileName();
/** Gets the path of the file this difference applies to */
public Path getPath();

/**
* Applies this difference to the supplied {@code sourceFile}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.AbstractService;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Set;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.ConcurrentSkipListSet;
Expand All @@ -40,8 +41,8 @@
public class DiffApplier extends AbstractService {
private static final Logger logger = Logger.getLogger(DiffApplier.class.getName());
private final ExecutorService workerService;
private final Set<String> refactoredPaths;
private final Set<String> diffsFailedPaths;
private final Set<Path> refactoredPaths;
private final Set<Path> diffsFailedPaths;
private final FileSource source;
private final FileDestination destination;
private final AtomicInteger completedFiles;
Expand All @@ -67,7 +68,7 @@ public DiffApplier(int diffParallelism, FileSource source, FileDestination desti
diffParallelism,
5,
TimeUnit.SECONDS,
new ArrayBlockingQueue<Runnable>(50),
new ArrayBlockingQueue<>(50),
new ThreadPoolExecutor.CallerRunsPolicy());
}

Expand Down Expand Up @@ -114,7 +115,7 @@ private final class Task implements Runnable {
@Override
public void run() {
try {
SourceFile file = source.readFile(diff.getRelevantFileName());
SourceFile file = source.readFile(diff.getPath());
diff.applyDifferences(file);
destination.writeFile(file);

Expand All @@ -123,16 +124,16 @@ public void run() {
logger.log(Level.INFO, String.format("Completed %d files in %s", completed, stopwatch));
}
} catch (IOException | DiffNotApplicableException e) {
logger.log(Level.WARNING, "Failed to apply diff to file " + diff.getRelevantFileName(), e);
diffsFailedPaths.add(diff.getRelevantFileName());
logger.log(Level.WARNING, "Failed to apply diff to file " + diff.getPath(), e);
diffsFailedPaths.add(diff.getPath());
} finally {
decrementTasks();
}
}
}

public Future<?> put(Diff diff) {
if (refactoredPaths.add(diff.getRelevantFileName())) {
if (refactoredPaths.add(diff.getPath())) {
runState.incrementAndGet();
return workerService.submit(new Task(diff));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
package com.google.errorprone.apply;

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

/** @author [email protected] (Simon Nickerson) */
public interface FileSource {

SourceFile readFile(String path) throws IOException;
SourceFile readFile(Path path) throws IOException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,13 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;

/** A {@link FileDestination} that writes content to a destination on the local filesystem. */
public final class FsFileDestination implements FileDestination {

private final Path rootPath;

public FsFileDestination(Path rootPath) {
this.rootPath = rootPath;
}

@Override
public void writeFile(SourceFile update) throws IOException {
Path targetPath = rootPath.resolve(update.getPath());
Files.write(targetPath, update.getSourceText().getBytes(StandardCharsets.UTF_8));
Files.write(update.getPath(), update.getSourceText().getBytes(StandardCharsets.UTF_8));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,8 @@
/** A FileSource that reads source files from the local filesystem. */
public final class FsFileSource implements FileSource {

private final Path rootPath;

public FsFileSource(Path rootPath) {
this.rootPath = rootPath;
}

@Override
public SourceFile readFile(String path) throws IOException {
return new SourceFile(
path, new String(Files.readAllBytes(rootPath.resolve(path)), StandardCharsets.UTF_8));
public SourceFile readFile(Path path) throws IOException {
return new SourceFile(path, new String(Files.readAllBytes(path), StandardCharsets.UTF_8));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.io.LineNumberReader;
import java.io.StringReader;
import java.nio.CharBuffer;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import javax.tools.JavaFileObject;
Expand All @@ -37,20 +39,26 @@
*/
public class SourceFile {

private final String path;
private final Path path;
private final StringBuilder sourceBuilder;

public static SourceFile create(JavaFileObject fileObject) throws IOException {
return new SourceFile(fileObject.toUri().getPath(), fileObject.getCharContent(false));
Path path;
try {
path = Paths.get(fileObject.toUri());
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(fileObject.toUri().toString(), e);
}
return new SourceFile(path, fileObject.getCharContent(false));
}

public SourceFile(String path, CharSequence source) {
public SourceFile(Path path, CharSequence source) {
this.path = path;
sourceBuilder = new StringBuilder(source);
}

/** Returns the path for this source file */
public String getPath() {
public Path getPath() {
return path;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
import com.sun.tools.javac.util.Options;
import com.sun.tools.javac.util.Position;
import java.io.IOException;
import java.nio.file.Paths;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -642,7 +643,7 @@ public static boolean compilesWithFix(Fix fix, VisitorState state) {
try {
fixSource =
new SourceFile(
modifiedFile.getName(),
Paths.get(modifiedFile.toUri()),
modifiedFile.getCharContent(false /*ignoreEncodingErrors*/));
} catch (IOException e) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import static org.junit.Assert.assertEquals;

import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List;
import org.junit.Before;
Expand All @@ -33,7 +35,7 @@
@RunWith(JUnit4.class)
public class SourceFileTest {

private static final String DUMMY_PATH = "java/com/google/foo/bar/FooBar.java";
private static final Path DUMMY_PATH = Paths.get("java/com/google/foo/bar/FooBar.java");
private static final String SOURCE_TEXT =
"// Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do\n"
+ "// eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,32 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.CharStreams;
import com.google.common.io.Resources;
import com.google.common.jimfs.Jimfs;
import com.google.errorprone.apply.SourceFile;
import com.google.testing.compile.JavaFileObjects;
import com.sun.source.tree.CompilationUnitTree;
import com.sun.tools.javac.api.JavacTaskImpl;
import com.sun.tools.javac.api.JavacTool;
import com.sun.tools.javac.file.JavacFileManager;
import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
import com.sun.tools.javac.tree.JCTree.JCMethodDecl;
import com.sun.tools.javac.tree.TreeInfo;
import com.sun.tools.javac.tree.TreeScanner;
import com.sun.tools.javac.util.Context;
import java.io.IOError;
import java.io.IOException;
import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Locale;
import java.util.Map;
import javax.tools.DiagnosticCollector;
import javax.tools.JavaCompiler;
import javax.tools.JavaFileObject;
import javax.tools.StandardJavaFileManager;
import org.junit.After;

/**
* Abstract skeleton for tests that run the compiler on the fly.
Expand All @@ -49,6 +58,8 @@ public class CompilerBasedTest {
protected SourceFile sourceFile;
protected Iterable<JCCompilationUnit> compilationUnits;
private Map<String, JCMethodDecl> methods;
private final JavacFileManager fileManager = new JavacFileManager(new Context(), false, UTF_8);
private final FileSystem fileSystem = Jimfs.newFileSystem();

protected void compile(TreeScanner scanner, JavaFileObject fileObject) {
JavaCompiler compiler = JavacTool.create();
Expand Down Expand Up @@ -107,10 +118,37 @@ public void visitTopLevel(JCCompilationUnit tree) {
}

protected void compile(String... lines) {
compile(JavaFileObjects.forSourceLines("CompilerBasedTestInput", lines));
try {
Path dir = fileSystem.getRootDirectories().iterator().next().resolve("tmp");
Files.createDirectories(dir);
Path path = Files.createTempFile(dir, "CompilerBasedTestInput", ".java");
Files.write(path, Arrays.asList(lines), UTF_8);
compile(fileManager.getJavaFileObject(path));
} catch (IOException e) {
throw new IOError(e);
}
}

JavaFileObject forResource(String name) {
try {
Path dir = fileSystem.getRootDirectories().iterator().next().resolve("tmp");
Files.createDirectories(dir);
Path path = Files.createTempDirectory(dir, "tmp").resolve(name);
Files.createDirectories(path.getParent());
Files.write(path, Resources.toByteArray(Resources.getResource(name)));
return fileManager.getJavaFileObject(path);
} catch (IOException e) {
throw new IOError(e);
}
}

protected JCMethodDecl getMethodDeclaration(String name) {
return methods.get(name);
}

@After
public void close() throws IOException {
fileSystem.close();
fileManager.close();
}
}
Loading

0 comments on commit 8a87d57

Please sign in to comment.