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

Shim class to make creating a formatter easier (also Spotless integration) #20

Closed
nedtwigg opened this issue Jan 10, 2017 · 7 comments
Closed

Comments

@nedtwigg
Copy link

Cool formatter! What do you think about adding something along these lines:

public class KtLintStandard {
    public KtLintStandard() {
        // setup the default rules, and a default callback function
    }

    public String format(String input) { ... }

Spotless is a formatting "switchboard" that composes different formatters for different languages with different build systems. It has some value-add like up-to-date checking and automatically fixing minor idempotence glitches. Spotless understands a formatting rule as a "Function<String, String>", and it integrates third-party formatters like google-java-format and eclipse via reflection.

I tried to integrate your library in spotless #67, but it's very hard to instantiate via reflection. A shim such as the one proposed above would make it a lot easier! Once this change is made, it could be as easy as

plugins { id 'com.diffplug.gradle.spotless' }
spotless { 
    kotlin {
        ktlint()   // takes an arg if they want to set a specific version
    }
}

For users to apply ktlint to their gradle projects with up-to-date-checking, and they could use other formatters for other languages. Also license enforcement, etc.

@shyiko
Copy link
Collaborator

shyiko commented Jan 10, 2017

Hi Ned,
That looks interesting!

I'm not entirely sure where would be the right place for this kind of class, though. Also without the callback user would have no way of knowing whether all violations have been corrected or some require manual intervention. That means that callback would have to stay, which in turn is going to make access through reflection awkward again.

One way to solve this would be to create a module with com.github.shyiko:ktlint "provided" (in Maven terminology) dependency that does no reflection at all and just acts as a bridge between spotless & ktlint. This module would obviously have to be part of the spotless project. What do you think?

On a different note, here is the code (using Reflection API) that works:

ClassLoader classLoader = ...

Class<?> ktlintClass = classLoader.loadClass("com.github.shyiko.ktlint.core.KtLint");
Object ktlint = ktlintClass.getDeclaredField("INSTANCE").get(null);

Class<?> standardRuleSetProviderClass = classLoader.loadClass("com.gihub.shyiko.ktlint.ruleset.standard.StandardRuleSetProvider");
Object standardRuleSet = standardRuleSetProviderClass.getMethod("get").invoke(standardRuleSetProviderClass.newInstance());
Iterable<?> ruleSets = Collections.singletonList(standardRuleSet);

Class<?> function2Interface = classLoader.loadClass("kotlin.jvm.functions.Function2");
Object formatterCallback = Proxy.newProxyInstance(classLoader, new Class[]{function2Interface},
    (proxy, method, args) -> null);

Method formatterMethod = ktlintClass.getMethod("format", String.class, Iterable.class, function2Interface);

String input = "fun main() { x(1,3);  x(1, 3)       }";
String output = (String) formatterMethod.invoke(ktlint, input, ruleSets, formatterCallback);

@nedtwigg
Copy link
Author

Spotless has an _ext folder for hosting shim modules, so that could be done. I don't yet have the Kotlin skill to build and maintain the shim, but I'd be happy to take a PR. Spotless has zero dependencies - it calls all its formatting libraries using reflection, but caches the classloaders to get the full JIT performance.

My immediate need is for Spotless, but I expect that other users of your formatting library would appreciate an easier entry point as well. In my eyes, the simplest interface for a formatter is a Function<String, String> that throws an exception if it can't generate an adequate return value. The easier it is for users to generate such a construct from your library, the easier it is to consume your library to meet a variety of usecases.

In the reflection code you pasted above, how does it handle "violations that require manual intervention"? Does it throw a descriptive exception? If not, is it possible to modify the pasted code so that it would throw a descriptive exception?

@shyiko
Copy link
Collaborator

shyiko commented Jan 10, 2017

You don't need Kotlin to make a shim. A little bit of Java will do:

try {
    output = KtLint.INSTANCE.format(input, Arrays.asList(new StandardRuleSetProvider().get()), (lintError, corrected) -> {
        if (!corrected) {
            throw new RuntimeException(lintError.getDetail());
        }
        return null;
    });
} catch (RuleExecutionException e) {
    System.err.println(e.getLine() + ":" + e.getCol() + " " + e.getMessage() + " (" + e.getRuleId() + ")");
}

In the reflection code you pasted above, how does it handle "violations that require manual intervention"?

It doesn't. But it's easy to correct:

Class<?> lintErrorClass = classLoader.loadClass("com.github.shyiko.ktlint.core.LintError");
Method detailGetter = lintErrorClass.getMethod("getDetail");
Object formatterCallback = Proxy.newProxyInstance(classLoader, new Class[]{function2Interface},
    (proxy, method, args) -> {
        Object lintError = args[0]; // com.github.shyiko.ktlint.core.LintError
        boolean corrected = (Boolean) args[1];
        if (!corrected) {
            String detail = (String) detailGetter.invoke(lintError);
            throw new RuntimeException(detail);
        }
        return null;
    });

...

try {
    output = (String) formatterMethod.invoke(ktlint, input, ruleSets, formatterCallback);
} catch (Exception e) {
   // InvocationTargetException -> RuleExecutionException (see prev code snippet)
}

nedtwigg added a commit to diffplug/spotless that referenced this issue Jan 11, 2017
@nedtwigg
Copy link
Author

Awesome, thanks! That got it working. Next question: can you point me to an example kotlin file that should throw an error? I'd like to add a test which asserts that errors are being thrown properly, and that their messages are as expected.

I think we're almost ready to merge ktlint into spotless :)

@shyiko
Copy link
Collaborator

shyiko commented Jan 11, 2017

Sure. This is an example of the rule that cannot be fixed automatically and this is the one that can (there are bunch of other examples here). Let me know if you need anything else.

@nedtwigg
Copy link
Author

Great, thanks! I think we're about ready to merge diffplug/spotless#71! Hopefully we'll be able to drive more users your way once it ships.

@shyiko
Copy link
Collaborator

shyiko commented Jan 11, 2017

@nedtwigg The feeling is mutual 🤝

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

No branches or pull requests

2 participants