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

Handle specifically empty filePath, and not default FS rootDir #1525

Merged
merged 11 commits into from
Jan 26, 2023
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ This document is intended for Spotless developers.
We adhere to the [keepachangelog](https://keepachangelog.com/en/1.0.0/) format (starting after version `1.27.0`).

## [Unreleased]
### Added
* `Formatter` now has a field `public static final File NO_FILE_SENTINEL` which can be used to pass string content to a Formatter or FormatterStep when there is no actual File to format. ([#1525](https://github.com/diffplug/spotless/pull/1525))

## [2.33.0] - 2023-01-26
### Added
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ VER_DURIAN=1.2.0
VER_JGIT=5.13.1.202206130422-r
VER_JUNIT=5.9.2
VER_ASSERTJ=3.24.2
VER_MOCKITO=4.11.0
VER_MOCKITO=4.11.0
14 changes: 11 additions & 3 deletions lib/src/main/java/com/diffplug/spotless/Formatter.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016 DiffPlug
* Copyright 2016-2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -237,8 +237,13 @@ public String compute(String unix, File file) {
unix = LineEnding.toUnix(formatted);
}
} catch (Throwable e) {
String relativePath = rootDir.relativize(file.toPath()).toString();
exceptionPolicy.handleError(e, step, relativePath);
if (file == NO_FILE_SENTINEL) {
exceptionPolicy.handleError(e, step, "");
} else {
// Path may be forged from a different FileSystem than Filesystem.default
String relativePath = rootDir.relativize(rootDir.getFileSystem().getPath(file.getPath())).toString();
exceptionPolicy.handleError(e, step, relativePath);
}
}
}
return unix;
Expand Down Expand Up @@ -284,4 +289,7 @@ public void close() {
}
}
}

/** This Sentinel reference may be used to pass string content to a Formatter or FormatterStep when there is no actual File to format */
public static final File NO_FILE_SENTINEL = new File("NO_FILE_SENTINEL");
}
6 changes: 3 additions & 3 deletions lib/src/main/java/com/diffplug/spotless/FormatterFunc.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2021 DiffPlug
* Copyright 2016-2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -127,7 +127,7 @@ public String apply(String unix, File file) throws Exception {

@Override
public String apply(String unix) throws Exception {
return apply(unix, FormatterStepImpl.SENTINEL);
return apply(unix, Formatter.NO_FILE_SENTINEL);
}
};
}
Expand Down Expand Up @@ -156,7 +156,7 @@ default String apply(String unix, File file) throws Exception {

@Override
default String apply(String unix) throws Exception {
return apply(unix, FormatterStepImpl.SENTINEL);
return apply(unix, Formatter.NO_FILE_SENTINEL);
}
}
}
6 changes: 3 additions & 3 deletions lib/src/main/java/com/diffplug/spotless/FormatterStep.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2021 DiffPlug
* Copyright 2016-2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -37,8 +37,8 @@ public interface FormatterStep extends Serializable {
* @param rawUnix
* the content to format, guaranteed to have unix-style newlines ('\n'); never null
* @param file
* the file which {@code rawUnix} was obtained from; never null. Pass an empty file using
* {@code new File("")} if and only if no file is actually associated with {@code rawUnix}
* the file which {@code rawUnix} was obtained from; never null. Pass the reference
* {@link Formatter#NO_FILE_SENTINEL} if and only if no file is actually associated with {@code rawUnix}
* @return the formatted content, guaranteed to only have unix-style newlines; may return null
* if the formatter step doesn't have any changes to make
* @throws Exception if the formatter step experiences a problem
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2022 DiffPlug
* Copyright 2016-2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -116,11 +116,8 @@ protected String format(Integer state, String rawUnix, File file) throws Excepti
}
}

/** A dummy SENTINEL file. */
static final File SENTINEL = new File("");

static void checkNotSentinel(File file) {
if (file == SENTINEL) {
if (file == Formatter.NO_FILE_SENTINEL) {
throw new IllegalArgumentException("This step requires the underlying file. If this is a test, use StepHarnessWithFile");
}
}
Expand Down
1 change: 1 addition & 0 deletions testlib/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ dependencies {
api "com.diffplug.durian:durian-testlib:${VER_DURIAN}"
api "org.junit.jupiter:junit-jupiter:${VER_JUNIT}"
api "org.assertj:assertj-core:${VER_ASSERTJ}"
api "org.mockito:mockito-core:$VER_MOCKITO"

implementation "com.diffplug.durian:durian-io:${VER_DURIAN}"
implementation "com.diffplug.durian:durian-collect:${VER_DURIAN}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public AbstractStringAssert<?> testResourceExceptionMsg(String resourceBefore) {

public AbstractStringAssert<?> testExceptionMsg(String before) {
try {
formatter.compute(LineEnding.toUnix(before), FormatterStepImpl.SENTINEL);
formatter.compute(LineEnding.toUnix(before), Formatter.NO_FILE_SENTINEL);
throw new SecurityException("Expected exception");
} catch (Throwable e) {
if (e instanceof SecurityException) {
Expand Down
98 changes: 97 additions & 1 deletion testlib/src/test/java/com/diffplug/spotless/FormatterTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2022 DiffPlug
* Copyright 2016-2023 DiffPlug
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,15 +15,20 @@
*/
package com.diffplug.spotless;

import java.io.File;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import com.diffplug.common.base.StandardSystemProperty;
import com.diffplug.spotless.generic.EndWithNewlineStep;
Expand Down Expand Up @@ -89,4 +94,95 @@ protected Formatter create() {
}
}.testEquals();
}

// new File("") as filePath is known to fail
@Test
public void testExceptionWithEmptyPath() throws Exception {
LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy();
Charset encoding = StandardCharsets.UTF_8;
FormatExceptionPolicy exceptionPolicy = FormatExceptionPolicy.failOnlyOnError();

Path rootDir = Paths.get(StandardSystemProperty.USER_DIR.value());

FormatterStep step = Mockito.mock(FormatterStep.class);
Mockito.when(step.getName()).thenReturn("someFailingStep");
Mockito.when(step.format(Mockito.anyString(), Mockito.any(File.class))).thenThrow(new IllegalArgumentException("someReason"));
List<FormatterStep> steps = Collections.singletonList(step);

Formatter formatter = Formatter.builder()
.lineEndingsPolicy(lineEndingsPolicy)
.encoding(encoding)
.rootDir(rootDir)
.steps(steps)
.exceptionPolicy(exceptionPolicy)
.build();

Assertions.assertThrows(IllegalArgumentException.class, () -> formatter.compute("someFileContent", new File("")));
}

// If there is no File actually holding the content, one may rely on Formatter.NO_FILE_ON_DISK
@Test
public void testExceptionWithSentinelNoFileOnDisk() throws Exception {
LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy();
Charset encoding = StandardCharsets.UTF_8;
FormatExceptionPolicy exceptionPolicy = FormatExceptionPolicy.failOnlyOnError();

Path rootDir = Paths.get(StandardSystemProperty.USER_DIR.value());

FormatterStep step = Mockito.mock(FormatterStep.class);
Mockito.when(step.getName()).thenReturn("someFailingStep");
Mockito.when(step.format(Mockito.anyString(), Mockito.any(File.class))).thenThrow(new IllegalArgumentException("someReason"));
List<FormatterStep> steps = Collections.singletonList(step);

Formatter formatter = Formatter.builder()
.lineEndingsPolicy(lineEndingsPolicy)
.encoding(encoding)
.rootDir(rootDir)
.steps(steps)
.exceptionPolicy(exceptionPolicy)
.build();

formatter.compute("someFileContent", Formatter.NO_FILE_SENTINEL);
}

// rootDir may be a path not from the default FileSystem
@Test
public void testExceptionWithRootDirIsNotFileSystem() throws Exception {
LineEnding.Policy lineEndingsPolicy = LineEnding.UNIX.createPolicy();
Charset encoding = StandardCharsets.UTF_8;
FormatExceptionPolicy exceptionPolicy = FormatExceptionPolicy.failOnlyOnError();

Path rootDir = Mockito.mock(Path.class);
FileSystem customFileSystem = Mockito.mock(FileSystem.class);
Mockito.when(rootDir.getFileSystem()).thenReturn(customFileSystem);

Path pathFromFile = Mockito.mock(Path.class);
Mockito.when(customFileSystem.getPath(Mockito.anyString())).thenReturn(pathFromFile);

Path relativized = Mockito.mock(Path.class);
Mockito.when(rootDir.relativize(Mockito.any(Path.class))).then(invok -> {
Path filePath = invok.getArgument(0);
if (filePath.getFileSystem() == FileSystems.getDefault()) {
throw new IllegalArgumentException("Can not relativize through different FileSystems");
}

return relativized;
});

FormatterStep step = Mockito.mock(FormatterStep.class);
Mockito.when(step.getName()).thenReturn("someFailingStep");
Mockito.when(step.format(Mockito.anyString(), Mockito.any(File.class))).thenThrow(new IllegalArgumentException("someReason"));
List<FormatterStep> steps = Collections.singletonList(step);

Formatter formatter = Formatter.builder()
.lineEndingsPolicy(lineEndingsPolicy)
.encoding(encoding)
.rootDir(rootDir)
.steps(steps)
.exceptionPolicy(exceptionPolicy)
.build();

formatter.compute("someFileContent", new File("/some/folder/some.file"));
}

}