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

Automatic Android support without specifying target "src/*/java/**/*.java" #111

Open
jaredsburrows opened this issue May 14, 2017 · 22 comments

Comments

@jaredsburrows
Copy link

jaredsburrows commented May 14, 2017

Does this support Android?


EDIT BY @nedtwigg, TOP COMMENT FOR VISIBILITY

On Android, your config file needs to look like this:

spotless {
  java {
    target "src/*/java/**/*.java"  // this line is needed
  }
}

We'd love a PR which fixes this, see this comment and this comment for what needs to be done.

@jbduncan
Copy link
Member

It should support Android. If it doesn't work with Android projects, I'd consider it a bug, in which case please let us know. :)

@jaredsburrows
Copy link
Author

jaredsburrows commented May 14, 2017

@jbduncan Why close this so fast?

I just tried this plugin on my project here: https://github.com/jaredsburrows/android-gif-example and it does seem to work.

Based on your README.md, are you expecting gradlew build? For Android, it is best for it to run on or after the assemble task.

@jaredsburrows
Copy link
Author

This does not work:

spotless {
  java {
    target "**/*.java"

    trimTrailingWhitespace()
    indentWithSpaces()
    endWithNewline()
  }
}

Even trying misc, does not work:

spotless {
  format "misc", {
    target "**/*.java"

    trimTrailingWhitespace()
    indentWithSpaces()
    endWithNewline()
  }
}

@jbduncan jbduncan reopened this May 14, 2017
@jbduncan
Copy link
Member

jbduncan commented May 14, 2017

@jbduncan Why close this so fast?

Oh, sorry! I thought that was the end, as I assumed you just had a question out of curiosity. My bad.

I just tried this plugin on my project here: https://github.com/jaredsburrows/android-gif-example and it does seem to work.

Glad to hear. 👍

Based on your README.md, are you excepting gradlew build? For Android, it is best for it to run on or after the assemble task.

By "excepting", did you mean "expecting"?

Yes, that's one of 3 ways one can use Spotless for non-Android Java projects:

  1. gradlew build
  2. gradlew check
  3. gradlew spotlessCheck

The reason all 3 of these tasks work (on non-Android Java builds at least) is because the build task implicitly depends on and runs check, which in turn runs spotlessCheck when Spotless is registered as a plugin.

From your example, I can't tell what about it doesn't work or why. Can you provide an minimal, verifiable, and complete example for us to try out?

@jaredsburrows
Copy link
Author

@jbduncan I gave you an example to try out already in the previous comment.

  1. git clone https://github.com/jaredsburrows/android-gif-example
  2. Apply your plugin to build.gradle (https://plugins.gradle.org/plugin/com.diffplug.gradle.spotless)
  3. gradlew spotlessCheck:
$ gradlew spotlessCheck
NDK is missing a "platforms" directory.
If you are using NDK, verify the ndk.dir is set to a valid NDK directory.  It is currently set to /Users/noname/android-sdk/ndk-bundle.
If you are not using NDK, unset the NDK variable from ANDROID_NDK_HOME or local.properties to remove this warning.

:spotlessCheck UP-TO-DATE

BUILD SUCCESSFUL

Total time: 5.84 secs

Nothing happens. I have even tried the following:

This does not work:

spotless {
  java {
    target "**/*.java"

    trimTrailingWhitespace()
    indentWithSpaces()
    endWithNewline()
  }
}

Even trying misc, does not work:

spotless {
  format "misc", {
    target "**/*.java"

    trimTrailingWhitespace()
    indentWithSpaces()
    endWithNewline()
  }
}

@jaredsburrows
Copy link
Author

jaredsburrows commented May 14, 2017

This worked for me now: gradlew spotlessApply.

For Android, you have to specify a target: target "**/*.java"

The java/java-library plugin expects sourceSets and the com.android.application plugin expects android.sourceSets. We have to manually specify the target sources.

spotless {
  java {
    target "**/*.java"
    googleJavaFormat()
    removeUnusedImports()
    trimTrailingWhitespace()
    indentWithSpaces()
    endWithNewline()
  }
}

@jbduncan
Copy link
Member

jbduncan commented May 15, 2017

@jaredsburrows Glad to hear to that spotlessApply works! Does spotlessCheck work on your end too?

It's annoying that you have to manually specify the target as target "**/*.java". I'm just in the middle of installing Android Studio so I have the Android SDK up and running to be able to execute the Gradle buildscript and see for myself what's wrong.

@nedtwigg I'm not terribly familiar with Spotless's internals yet, so do you have any thoughts on what we could do allow Spotless to support Android?

@jbduncan jbduncan reopened this May 15, 2017
@nedtwigg
Copy link
Member

The android plugin must not be friends with this code.

@jaredsburrows
Copy link
Author

@jbduncan Yes it does. You just have to specify apply.

@nedtwigg Android plugin/library has it's own source sets in it's DSL: android.sourceSets where the Java plugin has sourceSets.

@jbduncan
Copy link
Member

jbduncan commented May 16, 2017

@jaredsburrows Can you clarify for me what you mean when you say "You just have to specify apply"?

Just to make sure we're on the same page, I want to clarify that spotlessCheck and spotlessApply are independent tasks (in theory), where spotlessApply formats code (useful for a general software development flow), and spotlessCheck essentially throws an error if code is not formatted properly (very useful for enforcing a code standard on CI servers and such).

@jaredsburrows
Copy link
Author

@jbduncan I was talking about the target "**/*.java".

spotless {
  java {
    target "**/*.java"      <--- without this, it doesn't work with Android
    googleJavaFormat()
    removeUnusedImports()
    trimTrailingWhitespace()
    indentWithSpaces()
    endWithNewline()
  }
}

@nedtwigg
Copy link
Member

I added the workaround in the top comment for others to find, along with links to the places where the code would need to be fixed if anyone wants to submit a PR for this.

@cinemast
Copy link

cinemast commented May 31, 2017

@jaredsburrows @nedtwigg Does someone have a full example of where to embed the plugin into the android gradle files? The android-gif example does not include these sections. Thanks in advance.

I always wind up with a Error:org.gradle.internal.resolve.ModuleVersionNotFoundException: Could not find com.google.googlejavaformat:google-java-format:1.3.

@jaredsburrows
Copy link
Author

jaredsburrows commented May 31, 2017 via email

@cinemast
Copy link

I manged to solve it. I was missing what was mentioned in #103.

Maybe others find it useful, so I post my final solution here.
This is whats required in the top level gradle file for android projects.

// Top-level build file where you can add configuration options common to all sub-projects/modules.

buildscript {
    repositories {
        jcenter()
    }
    dependencies {
        classpath 'com.android.tools.build:gradle:2.3.1'

        // NOTE: Do not place your application dependencies here; they belong
        // in the individual module build.gradle files
    }
}

plugins {
    id "com.diffplug.gradle.spotless" version "3.4.0"
}

allprojects {
    repositories {
        jcenter()
    }

    buildscript {
        repositories {
            maven { url "https://plugins.gradle.org/m2/" }
        }
    }

    apply plugin: 'com.diffplug.gradle.spotless'

    spotless {
        java {
            target "**/*.java"
            trimTrailingWhitespace()
            removeUnusedImports()
            googleJavaFormat()
        }
    }
}

task clean(type: Delete) {
    delete rootProject.buildDir
}

@cinemast
Copy link

@nedtwigg maybe you could add this to the documentation somewhere?

@jbduncan
Copy link
Member

jbduncan commented May 31, 2017

@cinemast I believe your example could be shortened. Does the build file below work?

// Top-level build file where you can add configuration options common to all sub-projects/modules.

buildscript {
    repositories {
        jcenter()
    }
    dependencies {
        classpath 'com.android.tools.build:gradle:2.3.1'

        // NOTE: Do not place your application dependencies here; they belong
        // in the individual module build.gradle files
    }
}

plugins {
    id "com.diffplug.gradle.spotless" version "3.4.0"
}

allprojects {
    buildscript {
        repositories {
            jcenter() // to allow Spotless to download formatters
        }
    }

    // @cinemast "apply plugin: ..." shouldn't be needed because it is already applied implicitly through the "plugin {}" block above.

    spotless {
        java {
            target "**/*.java"
            trimTrailingWhitespace()
            removeUnusedImports()
            googleJavaFormat()
        }
    }
}

// @cinemast No clean task should be needed, I think? (In non-Android Java projects, clean automatically exists and cleans the root project's build directory.)

@runningcode
Copy link
Contributor

I required the apply plugin and target "**/*.java" parts in my android project as well.

@JLLeitschuh
Copy link
Member

The target "**/*.java" is currently the cleanest solution without adding a dependency upon the gradle android plugin inside of spotless.

When you start having plugins dependent upon other plugins you end up tied to their breaking changes version bumping.

I've had to deal with this in my own plugin.
JLLeitschuh/ktlint-gradle#32

@runningcode
Copy link
Contributor

That's fine. Where can/should we document this?

@JLLeitschuh
Copy link
Member

Probably the plugin README

runningcode pushed a commit to runningcode/spotless that referenced this issue Nov 27, 2017
runningcode pushed a commit to runningcode/spotless that referenced this issue Nov 27, 2017
runningcode pushed a commit to runningcode/spotless that referenced this issue Nov 27, 2017
@nedtwigg nedtwigg changed the title Android/Java support with Android Gradle Plugin Automatic Android support without specifying target "**/*.java" Jan 13, 2018
@nedtwigg nedtwigg changed the title Automatic Android support without specifying target "**/*.java" Automatic Android support without specifying target "src/*/java/**/*.java" Jun 30, 2020
@nedtwigg
Copy link
Member

Just a bump that src/*/java/**/*.java will perform far better than **/*.java. We heavily discourage **/blah wildcards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants