Unexpected behaviour from `indentWithSpaces(int)`

[jbduncan]

Given the following build script and Java file on Gradle 3.2.1:
build.gradle

plugins {
    id 'java'
    id 'com.diffplug.gradle.spotless' version '2.4.0'
}

group 'org.jbduncan'
version '1.0-SNAPSHOT'

sourceCompatibility = 1.8

repositories {
    mavenCentral()
}

spotless {
    format('example') {
        target '**/*.java'
        indentWithSpaces(4)
    }
}

src/main/java/HelloWorld.java

public final class HelloWorld {
      public String greeting() {
        return "Hello, world!";
    }
}

...I would have expected the Java file to turn into:

public final class HelloWorld {
    public String greeting() {
        return "Hello, world!";
    }
}

or:

public final class HelloWorld {
        public String greeting() {
        return "Hello, world!";
    }
}

...but it seems it doesn't.

Have I misunderstood what indentWithSpaces(4) is supposed to do here? I understand that it turns all tab indents into 4-space indents, but by my intuitive understanding it should also make sure that all space-indents have a number of spaces equal to a multiple of 4.

[nedtwigg]

At first glance, the behavior you expect would be better than the behavior it actually has. But consider this case:

/**
 * An implementation of this class specifies a single step in a formatting process.
 *
 * The input is guaranteed to have unix-style newlines, and the output is required
 * to not introduce any windows-style newlines as well.
 */
public interface FormatterStep extends Serializable {
    ...
}

The behavior you expect would clobber those leading spaces, which seem to be the appropriate formatting even for indentWithTabs(). Many languages with block comments have indentation exceptions which are difficult to code in a general-purpose way.

So the current behavior is to just ensure that if the formatting looks right, then it is right. Tabs and spaces are invisible, so you can't see if you used the right one. With the current behavior, so long as it looks right, spotless will make sure that you used the right characters, and if you used the wrong one, it can swap it for you automatically.

I'm open to a more powerful compromise if we think of one :)

[jbduncan]

That sounds very reasonable to me, especially as I can now see how my idea would indeed mangle Javadoc comments and the like.

I don't have any ideas for a more powerful compromise, at least not yet, so I'm happy that this issue is left open until we either find such an idea or think it's no longer with pursuing. :)

[nedtwigg]

If anyone has an idea, feel free to reopen :)

[kekko1212]

I guess one option could be to make the parser understand that it is within a comment block, therefore all leading spaces/ tab should be ignored or treated using a sensible default. Would that something feasible?

[nedtwigg]

We do something vaguely similar with the LicenseHeaderStep. Its parameters include a regex to find the "top" of the file, then the license you want. For each language, we have a language-specific regex which gets passed along transparently.

If someone wants to contribute something similar for this, that'd be great! I worry that it will turn out to be "edge-case hell", but maybe it turns out to be simple. I'll reopen when/if we get plausible PR with good testcases. It's also critical to maintain back-compat.

[kekko1212]

Yeah I was thinking to help tbh. I have a lot of spare time these days, so, why not contributing to OS? 😄

Thanks for the info, I'll see if I can get a PR anytime soon. Would be my pleasure.

[wheelerlaw]

It seems that the root of the issue here is that Spotless doesn't do AST parsing to determine that it's in a comment block or not. Since it seems like what @nedtwigg is describing is that Spotless has no way of knowing it is in a block comment since it making corrections on a line by line basis?

[nedtwigg]

Spotless is a switchboard for other formatters. Some formatters do AST parsing, some don't. Spotless let's you take a formatter which does do AST parsing (e.g. eclipse), and then modify that result with something that doesn't (e.g. indentWithTabs.