Skip to content

Adding npm-based formatters typescript-formatter (tsfmt) and prettier #283

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.

Sign up for GitHub

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

Merged
merged 46 commits into from
Sep 24, 2018
Merged

Adding npm-based formatters typescript-formatter (tsfmt) and prettier #283

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
75ce9aa
initial implementation of nodebased steps
simschla Sep 7, 2018
dfe881d
Move tests for into lib-extra.
nedtwigg Aug 20, 2018
c0bb96f
Separate the `NpmTest` tests from the rest of the test suite.
nedtwigg Aug 20, 2018
991457c
renaming package nodebased -> npm
simschla Sep 7, 2018
164f047
move subpackages to npm top-level package
simschla Sep 7, 2018
37fff43
restrict visibility
simschla Sep 7, 2018
ddfc882
revert the from-file feature for JarState as it is not used right now
simschla Sep 7, 2018
ef4d244
adapt to package renaming
simschla Sep 7, 2018
4f16998
using map as config api for npm formatter steps
simschla Sep 7, 2018
74dfaee
clearing up resources after prettier/tsfmt run
simschla Sep 7, 2018
e706531
spotbugs cleanup
simschla Sep 7, 2018
b8fb907
cleanup autorelease-code
simschla Sep 7, 2018
11b32f4
restrict engine versions according to the npm packages used
simschla Sep 7, 2018
c02b3ef
dynamically select j2v8 lib based on platform
simschla Sep 7, 2018
27dc6f9
add support for closing FormatterFunc
simschla Sep 7, 2018
0b7f6a9
implement behavioral tests
simschla Sep 7, 2018
4b25b7c
validate working config options for tsfmt
simschla Sep 7, 2018
48430c8
normalized arch name (matching arch for for ci)
simschla Sep 7, 2018
ff1d3d3
validate multiple formatting filetypes are working
simschla Sep 7, 2018
d832ccd
test prettier configuration behavior
simschla Sep 7, 2018
ff9a3d2
auto-resolve npm binary (at least for tests very useful)
simschla Sep 7, 2018
3dfe3c9
adding gradle-integration for prettier
simschla Sep 7, 2018
d21f2bc
adding gradle-integration for tsfmt
simschla Sep 7, 2018
8b1cf97
Fix minor warnings in eclipse.
nedtwigg Sep 4, 2018
18c7748
Made as much of the npm implementation package-private as possible.
nedtwigg Sep 4, 2018
5550174
Fixed the gradle typescript extension.
nedtwigg Sep 4, 2018
092a488
Make the npm tests run on CI.
nedtwigg Sep 4, 2018
83178d1
Moved all of npm's test files into testlib so that they can be reused…
nedtwigg Sep 4, 2018
ded5885
readFileFromClasspath is now compatible with the gradle classpath
nedtwigg Sep 4, 2018
4ee647e
Added an integration test for TypescriptExtension, but it doesn't pas…
nedtwigg Sep 4, 2018
0b604ab
A super incomplete version of the docs, to plant the seed.
nedtwigg Sep 4, 2018
2197c27
first documentation draft
simschla Sep 7, 2018
732e27a
unify naming
simschla Sep 7, 2018
e400fa5
provide default basedir
simschla Sep 7, 2018
1fb5838
step 1 on supporting inline-config for typescript-formatter
simschla Sep 7, 2018
b5e57e7
introduce inline-config for tsfmt step
simschla Sep 7, 2018
2a21254
adding more integration tests for typescript extension
simschla Sep 7, 2018
3af839f
integration tests for prettier formatter
simschla Sep 7, 2018
c3c8ae1
unify usage of NpmTest category
simschla Sep 7, 2018
f817309
PR feedback: extract into sub-methods
simschla Sep 8, 2018
bf16cab
update documentation for tsfmt config file part
simschla Sep 9, 2018
3d3f304
smaller api for tsfmt config files
simschla Sep 11, 2018
6e07578
moving npm based formatters from lib-extra to lib
simschla Sep 12, 2018
ade884a
fixes for running on windows
simschla Sep 12, 2018
807bf7b
integrating PR feedback
simschla Sep 13, 2018
60f2bcd
Merge branch 'master' into feature/typescript-formatting-using-nodejs…
simschla Sep 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
clearing up resources after prettier/tsfmt run
  • Loading branch information
@simschla
simschla committed Sep 7, 2018
commit 74dfaeef836fd87de097ee578fbdfa2fc8bd1ceb
19 changes: 10 additions & 9 deletions lib-extra/src/main/java/com/diffplug/spotless/extra/npm/PrettierFormatterStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,16 @@ public FormatterFunc createFormatterFunc() {
try (
V8FunctionWrapper resolveConfigCallback = nodeJSWrapper.createNewFunction((receiver, parameters) -> {
Copy link
Member

@JLLeitschuh JLLeitschuh Sep 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's some wacky formatting here

Can I suggest you put some of this logic into outer functions?

Copy link
Contributor Author

@simschla simschla Sep 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed! just pushed a change to extract some of the cluttering object creation statements

try {
final V8ObjectWrapper configOptions = parameters.getObject(0); // why is this always null
if (configOptions == null) {
toThrow[0] = new IllegalArgumentException("Cannot find or read config file " + this.prettierConfig.getPrettierConfigPath());
} else {
Map<String, Object> resolvedOptions = new TreeMap<>(V8ObjectUtilsWrapper.toMap(configOptions));
resolvedOptions.putAll(this.prettierConfig.getOptions());
toThrow[0] = validateOptions(resolvedOptions);
if (toThrow[0] == null) {
resolvedPrettierOptions[0] = resolvedOptions;
try (final V8ObjectWrapper configOptions = parameters.getObject(0)) {
if (configOptions == null) {
toThrow[0] = new IllegalArgumentException("Cannot find or read config file " + this.prettierConfig.getPrettierConfigPath());
} else {
Map<String, Object> resolvedOptions = new TreeMap<>(V8ObjectUtilsWrapper.toMap(configOptions));
resolvedOptions.putAll(this.prettierConfig.getOptions());
toThrow[0] = validateOptions(resolvedOptions);
if (toThrow[0] == null) {
resolvedPrettierOptions[0] = resolvedOptions;
}
}
}
} catch (Exception e) {
Expand Down
24 changes: 13 additions & 11 deletions lib-extra/src/main/java/com/diffplug/spotless/extra/npm/TsFmtFormatterStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@

import javax.annotation.Nonnull;

import com.diffplug.common.collect.ImmutableList;
import com.diffplug.spotless.FormatterFunc;
import com.diffplug.spotless.FormatterStep;
import com.diffplug.spotless.Provisioner;
import com.diffplug.spotless.ThrowingEx;

public class TsFmtFormatterStep {

Expand Down Expand Up @@ -71,10 +71,14 @@ public FormatterFunc createFormatterFunc() {
final V8ObjectWrapper formatterOptions = nodeJSWrapper.createNewObject(tsFmtOptions);

final TsFmtResult[] tsFmtResult = new TsFmtResult[1];
final Exception[] toThrow = new Exception[1];

V8FunctionWrapper formatResultCallback = nodeJSWrapper.createNewFunction((receiver, parameters) -> {
final V8ObjectWrapper result = parameters.getObject(0);
tsFmtResult[0] = new TsFmtResult(result.getString("message"), result.getBoolean("error"), result.getString("dest"));
//result.release(); // TODO (simschla, 09.08.18): verify if release needed?
try (final V8ObjectWrapper result = parameters.getObject(0)) {
tsFmtResult[0] = new TsFmtResult(result.getString("message"), result.getBoolean("error"), result.getString("dest"));
} catch (Exception e) {
toThrow[0] = e;
}
return receiver;
});

Expand Down Expand Up @@ -103,10 +107,14 @@ public FormatterFunc createFormatterFunc() {

promise.executeVoidFunction("then", callbacks);

while (tsFmtResult[0] == null) {
while (tsFmtResult[0] == null && toThrow[0] == null) {
nodeJSWrapper.handleMessage();
}

if (toThrow[0] != null) {
throw ThrowingEx.asRuntime(toThrow[0]);
}

if (tsFmtResult[0] == null) {
throw new IllegalStateException("should never happen");
}
Expand All @@ -115,12 +123,6 @@ public FormatterFunc createFormatterFunc() {
}
return tsFmtResult[0].getFormatted();
}

// TODO (simschla, 09.08.18): release
// callbacks.release();
// args.release();
// promise.release();

});
}

Expand Down
14 changes: 8 additions & 6 deletions lib-extra/src/test/java/com/diffplug/spotless/extra/npm/PrettierFormatterStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,16 @@ ImmutableMap.<String, Object> builder()
.put("bracketSpacing", Boolean.TRUE)
.build()));

final Formatter formatter = Formatter.builder()
try (final Formatter formatter = Formatter.builder()
.encoding(StandardCharsets.UTF_8)
.rootDir(new File("/Users/simschla/tmp/demo-basedir").toPath())
.lineEndingsPolicy(LineEnding.UNIX.createPolicy())
.steps(Arrays.asList(formatterStep))
.exceptionPolicy(FormatExceptionPolicy.failOnlyOnError())
.build();
.build()) {

System.out.println("formatted: " + formatter.applyToAndReturnResultIfDirty(new File("/Users/simschla/tmp/demo-basedir", "example.ts")));
System.out.println("formatted: " + formatter.applyToAndReturnResultIfDirty(new File("/Users/simschla/tmp/demo-basedir", "example.ts")));
}
}

@Test
Expand All @@ -67,15 +68,16 @@ ImmutableMap.<String, Object> builder()
.put("parser", "json")
.build()));

final Formatter formatter = Formatter.builder()
try (final Formatter formatter = Formatter.builder()
.encoding(StandardCharsets.UTF_8)
.rootDir(new File("/Users/simschla/tmp/demo-basedir").toPath())
.lineEndingsPolicy(LineEnding.UNIX.createPolicy())
.steps(Arrays.asList(formatterStep))
.exceptionPolicy(FormatExceptionPolicy.failOnlyOnError())
.build();
.build()) {

System.out.println("formatted: " + formatter.applyToAndReturnResultIfDirty(new File("/Users/simschla/tmp/demo-basedir", "toformat.json")));
System.out.println("formatted: " + formatter.applyToAndReturnResultIfDirty(new File("/Users/simschla/tmp/demo-basedir", "toformat.json")));
}
}

}
7 changes: 4 additions & 3 deletions lib-extra/src/test/java/com/diffplug/spotless/extra/npm/TsFmtFormatterStepTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,15 @@ ImmutableMap.<String, Object> builder()
.put("tslint", Boolean.TRUE)
.build());

final Formatter formatter = Formatter.builder()
try (final Formatter formatter = Formatter.builder()
.encoding(StandardCharsets.UTF_8)
.rootDir(new File("/Users/simschla/tmp/demo-basedir").toPath())
.lineEndingsPolicy(LineEnding.UNIX.createPolicy())
.steps(Arrays.asList(formatterStep))
.exceptionPolicy(FormatExceptionPolicy.failOnlyOnError())
.build();
.build()) {

System.out.println("formatted: " + formatter.applyToAndReturnResultIfDirty(new File("/Users/simschla/tmp/demo-basedir", "example.ts")));
System.out.println("formatted: " + formatter.applyToAndReturnResultIfDirty(new File("/Users/simschla/tmp/demo-basedir", "example.ts")));
}
}
}