From 47ec9346f17cc6ac3c5f39109ee5ea754ce9577e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 22 Sep 2019 21:41:39 -0700 Subject: [PATCH 1/4] Bump for latest ide version. --- build.gradle | 2 +- ide/build.gradle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 6c5eef0aed..106a6ab335 100644 --- a/build.gradle +++ b/build.gradle @@ -2,7 +2,7 @@ plugins { // dependencyUpdates https://github.com/ben-manes/gradle-versions-plugin/releases id 'com.github.ben-manes.versions' version '0.22.0' // https://github.com/diffplug/goomph/blob/master/CHANGES.md - id 'com.diffplug.gradle.eclipse.resourcefilters' version '3.18.0' + id 'com.diffplug.gradle.eclipse.resourcefilters' version '3.18.1' // https://plugins.gradle.org/plugin/com.gradle.plugin-publish id 'com.gradle.plugin-publish' version '0.10.1' apply false // https://plugins.gradle.org/plugin/com.gradle.build-scan diff --git a/ide/build.gradle b/ide/build.gradle index 73888da83d..0a7877a231 100644 --- a/ide/build.gradle +++ b/ide/build.gradle @@ -12,6 +12,6 @@ oomphIde { thirdParty { minimalistGradleEditor {} - tmTerminal {} + //tmTerminal {} } } From ef972b241bbc9c078ea7866396c3ceb609cf7ac7 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 23 Sep 2019 09:31:10 -0700 Subject: [PATCH 2/4] Add a test to PaddedCellTaskTest which checks a well-behaved formatter, which shows the problem: - if a file is formatted badly - but the step itself is well-behaving on this file (is idempotent) - then Spotless will not mark the failure Effectively, paddedCell() was accidentally giving a pass to badly-formatted files, but only if their formatter was well-behaved on that particular file. --- .../diffplug/gradle/spotless/PaddedCellTaskTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java index a3d97c3351..47dfb48c4a 100644 --- a/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java +++ b/plugin-gradle/src/test/java/com/diffplug/gradle/spotless/PaddedCellTaskTest.java @@ -90,6 +90,10 @@ private String checkFailureMsg() { } } + private Bundle wellbehaved() throws IOException { + return new Bundle("wellbehaved", x -> "42"); + } + private Bundle cycle() throws IOException { return new Bundle("cycle", x -> x.equals("A") ? "B" : "A"); } @@ -104,6 +108,7 @@ private Bundle diverge() throws IOException { @Test public void failsWithoutPaddedCell() throws IOException { + Assertions.assertThat(wellbehaved().checkFailureMsg()).startsWith("The following files had format violations"); Assertions.assertThat(cycle().checkFailureMsg()).startsWith("You have a misbehaving rule"); Assertions.assertThat(converge().checkFailureMsg()).startsWith("You have a misbehaving rule"); Assertions.assertThat(diverge().checkFailureMsg()).startsWith("You have a misbehaving rule"); @@ -111,18 +116,22 @@ public void failsWithoutPaddedCell() throws IOException { @Test public void paddedCellApply() throws Exception { + Bundle wellbehaved = wellbehaved().paddedCell(); Bundle cycle = cycle().paddedCell(); Bundle converge = converge().paddedCell(); Bundle diverge = diverge().paddedCell(); + execute(wellbehaved.apply); execute(cycle.apply); execute(converge.apply); execute(diverge.apply); + assertFile(wellbehaved.file).hasContent("42"); // cycle -> first element in cycle assertFile(cycle.file).hasContent("A"); // cycle -> first element in cycle assertFile(converge.file).hasContent(""); // converge -> converges assertFile(diverge.file).hasContent("CCC"); // diverge -> no change + execute(wellbehaved.check); execute(cycle.check); execute(converge.check); execute(diverge.check); @@ -130,6 +139,7 @@ public void paddedCellApply() throws Exception { @Test public void paddedCellCheckFailureFiles() throws Exception { + wellbehaved().paddedCell().checkFailureMsg(); cycle().paddedCell().checkFailureMsg(); converge().paddedCell().checkFailureMsg(); execute(diverge().paddedCell().check); From 0427f1140333f18134b88dac8c41b5e2129e27ed Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 23 Sep 2019 09:32:18 -0700 Subject: [PATCH 3/4] Fixed the bug in PaddedCellBulk. --- .../com/diffplug/spotless/PaddedCellBulk.java | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java b/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java index 6037146058..1d37ea72dc 100644 --- a/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java +++ b/lib/src/main/java/com/diffplug/spotless/PaddedCellBulk.java @@ -143,18 +143,18 @@ public static List check(File rootDir, File diagnoseDir, Formatter formatt Files.write(path, version.getBytes(formatter.getEncoding())); } // dump the type of the misbehavior to console - logger.finer(" " + relative + " " + padded.userMessage()); + logger.fine(" " + relative + " " + padded.userMessage()); + } - if (!padded.isResolvable()) { - // if it's not resolvable, then there's - // no point killing the build over it - } else { - // if the input is resolvable, we'll use that to try again at - // determining if it's clean - paddedCellStep.set(problemFile, padded.canonical()); - if (!paddedFormatter.isClean(problemFile)) { - stillFailing.add(problemFile); - } + if (!padded.isResolvable()) { + // if it's not resolvable, then there's + // no point killing the build over it + } else { + // if the input is resolvable, we'll use that to try again at + // determining if it's clean + paddedCellStep.set(problemFile, padded.canonical()); + if (!paddedFormatter.isClean(problemFile)) { + stillFailing.add(problemFile); } } } From 60eadd90a7bb26357f43602fd09f1ce0e12766a4 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Mon, 23 Sep 2019 10:15:59 -0700 Subject: [PATCH 4/4] Update changelog. --- CHANGES.md | 6 ++++++ plugin-gradle/CHANGES.md | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index e565d5d961..b9ec179345 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,6 +11,12 @@ You might be looking for: * Fixes [#410](https://github.com/diffplug/spotless/issues/410) AccessDeniedException in MinGW/ GitBash. * Also fixes occasional [hang on NFS due to filesystem timers](https://github.com/diffplug/spotless/pull/407#issuecomment-514824364). * Eclipse-based formatters used to leave temporary files around ([#447](https://github.com/diffplug/spotless/issues/447)). This is now fixed, but only for eclipse 4.12+, no back-port to older Eclipse formatter versions is planned. ([#451](https://github.com/diffplug/spotless/issues/451)) +* `PaddedCellBulk` had a bug where badly-formatted files with well-behaving formatters were being: + - correctly formatted by "apply" + - but incorrectly marked as good by "check" + - this led to "check" says all good, but then "apply" still causes format (https://github.com/diffplug/spotless/issues/453) + - combined with up-to-date checking, could lead to even more confusing results (https://github.com/diffplug/spotless/issues/338) + - only affects the gradle plugin, since that was the only plugin to use this feature ### Version 1.24.1 - August 12th 2018 (javadoc [lib](https://diffplug.github.io/spotless/javadoc/spotless-lib/1.24.1/) [lib-extra](https://diffplug.github.io/spotless/javadoc/spotless-lib-extra/1.24.1/), artifact [lib]([jcenter](https://bintray.com/diffplug/opensource/spotless-lib), [lib-extra]([jcenter](https://bintray.com/diffplug/opensource/spotless-lib-extra))) diff --git a/plugin-gradle/CHANGES.md b/plugin-gradle/CHANGES.md index 8facb582f2..888f70e0c5 100644 --- a/plugin-gradle/CHANGES.md +++ b/plugin-gradle/CHANGES.md @@ -6,6 +6,13 @@ * Fixes [#410](https://github.com/diffplug/spotless/issues/410) AccessDeniedException in MinGW/GitBash. * Also fixes occasional [hang on NFS due to filesystem timers](https://github.com/diffplug/spotless/pull/407#issuecomment-514824364). * Eclipse-based formatters used to leave temporary files around ([#447](https://github.com/diffplug/spotless/issues/447)). This is now fixed, but only for eclipse 4.12+, no back-port to older Eclipse formatter versions is planned. ([#451](https://github.com/diffplug/spotless/issues/451)) +* Fixed a bad but simple bug in `paddedCell()` (https://github.com/diffplug/spotless/pull/455) + - if a formatter was behaving correctly on a given file (was idempotent) + - but the file was not properly formatted + - `spotlessCheck` would improperly say "all good" even though `spotlessApply` would properly change them + - combined with up-to-date checking, could lead to even more confusing results, + (https://github.com/diffplug/spotless/issues/338) + - Fixed now! ### Version 3.24.2 - August 19th 2019 ([javadoc](https://diffplug.github.io/spotless/javadoc/spotless-plugin-gradle/3.24.1/), [jcenter](https://bintray.com/diffplug/opensource/spotless-plugin-gradle/3.24.1))