Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Allow passing options to formatters #4281

Closed
JoshuaKGoldberg opened this issue Nov 9, 2018 · 3 comments
Closed

Allow passing options to formatters #4281

JoshuaKGoldberg opened this issue Nov 9, 2018 · 3 comments

Comments

@JoshuaKGoldberg
Copy link
Contributor

Related: #4223 for whether prose (soon, verbose) should output relative or full file paths.

It could be useful to be able to pass options to formatters. For example, if we default to either relative or full file paths, some users may want the other behavior.

Instead of just taking in failures and fixes sequentially:

 interface IFormatter {
    format(failures: RuleFailure[], fixes?: RuleFailure[]): string;
}

It'd be nice to pass formatters an object with those fields and an options. Something like:

interface IFormatOptions<TOptions> {
    failures: RuleFailure[];
    fixes?: RuleFailure[];
    options: TOptions;
}

interface IFormatter<TOptions = {}> {
    format(formatOptions: IFormatOptions<TOptions>): string;
}

...but to maintain backwards compatibility with older TSLint versions, we'd want to still pass in a RuleFailure[] and fixes?: RuleFailure[]... so:

type IFormatOptions<TOptions> = RuleFailure[] & {
    failures: RuleFailure[];
    fixes?: RuleFailure[];
    options: TOptions;
};

interface IFormatter<TOptions = {}> {
    format(formatOptions: IFormatOptions<TOptions>, fixes?: RuleFailure[]): string;
}

...where the first object is an array with the failures, fixes, and options members added to it. The second parameter, fixes, would be deprecated, and we would want to eventually get rid of it.

@reduckted
Copy link
Contributor

Any thoughts on how (or if) the options would be specified on the command line?

@reduckted
Copy link
Contributor

Here's a prototype I've put together (and a comparison showing the changes I've made, and the specific commit that added the command line parsing).

I've gone with formatter options being specified on the command line as --format-*. Any arguments that match that format get turned into properties on an object that's passed to the formatter. For example, passing:

--format-foo-bar test --format-meep

will result in formatter options that look like:

{
    fooBar: "test",
    meep: true
}

It currently has the limitation that arrays are not supported (only string and boolean options work), though I might be able to fix that.

I tried strongly typing the options, and while it worked, it produced lint failures:

const formatter = new Formatter();
               // ~~~~~~~~~~~~~~~
               // no-inferred-empty-object-type
               // Explicit type parameter needs to be provided to the constructor

The problem is, because the formatters are dynamically created, we don't know the type of the options. I guess we could just disable that lint rule for that line, but the same error pops up in all of the tests, and even though we know the type of formatter being created, I couldn't work out how to fix the error.

before(() => {
    const Formatter = TestUtils.getFormatter("checkstyle");
    sourceFile1 = TestUtils.getSourceFile(TEST_FILE_1);
    sourceFile2 = TestUtils.getSourceFile(TEST_FILE_2);
    formatter = new Formatter();
             // ~~~~~~~~~~~~~~~
             // no-inferred-empty-object-type
             // Explicit type parameter needs to be provided to the constructor
});

When I do specify the type parameters, I get a compiler error:

before(() => {
    const Formatter = TestUtils.getFormatter("checkstyle");
    sourceFile1 = TestUtils.getSourceFile(TEST_FILE_1);
    sourceFile2 = TestUtils.getSourceFile(TEST_FILE_2);
    formatter = new Formatter<{}>();
                           // ~~
                           // TS2558: Expected 0 type arguments, but got 1.
});

Maybe I'm just doing something wrong. ¯\_(ツ)_/¯

I've also added options to the prose and stylish just to prove that it works.

tslint --format prose --format-relative-paths

The configuration file defines a linterOptions.format property, so that would be a good place to add a formatterOptions property, but I haven't gone that far with it yet.

If this way of getting options from the command line sounds like a good idea, I'll go ahead and formalize it into a proposal.

@JoshuaKGoldberg
Copy link
Contributor Author

💀 It's time! 💀

TSLint is being deprecated and no longer accepting pull requests for major new changes or features. See #4534. 😱

If you'd like to see this change implemented, you have two choices:

  • Recommended: Check if this is available in ESLint + typescript-eslint
  • Not Recommended: Fork TSLint locally 🤷‍♂️

👋 It was a pleasure open sourcing with you!

If you believe this message was posted here in error, please comment so we can re-open the issue!

@palantir palantir locked and limited conversation to collaborators Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants