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

Restricted Linter options type #1168

Merged
merged 9 commits into from
Apr 28, 2016
Merged

Restricted Linter options type #1168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
54359f9
Merge remote-tracking branch 'refs/remotes/palantir/master'
Apr 22, 2016
d6857cf
Merge remote-tracking branch 'refs/remotes/palantir/master'
Apr 26, 2016
8d79b52
Added check for non-object linter options
Apr 26, 2016
2f4909d
Changed linter options to allow odd inputs
Apr 27, 2016
54fe0fb
Merge remote-tracking branch 'refs/remotes/palantir/master'
Apr 27, 2016
87ca51f
Merge branch 'master' of https://github.com/JoshuaKGoldberg/tslint in…
Apr 27, 2016
9130842
Removed unnecessary npm-debug.log
Apr 27, 2016
d13594f
Slight linting rule fixups
Apr 27, 2016
9f69969
Slightly changed default linter settings
Apr 28, 2016
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
10 changes: 8 additions & 2 deletions src/lint.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,15 @@ export interface LintResult {
output: string;
}

export interface ILinterOptions {
export interface ILinterOptionsRaw {
Copy link
Contributor

Choose a reason for hiding this comment

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

Mixed feelings about if we really need a new interface here - I'm sort of inclined to just making the properties of ILinterOptions optional. What do you think?

Copy link
Contributor Author

@JoshuaKGoldberg JoshuaKGoldberg Apr 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

There's a TypeScript issue filed somewhere for letting us do something like ILinterOptions?, where every field in ILinterOptions would be marked optional (so exactly this).

My problem with re-using one interface for this is that after this initial parsing, the fields aren't all optional. The semantic intent changes from input option settings to stored options data, so IMO the type should as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me, let's keep this then. It shouldn't cause any backwards compat issues for library users as fulfilling the previous ILinterOptions interface will still fulfill the ILinterOptionsRaw interface. Maybe we should make the formattersDirectory field optional on both though, since it really is optional

configuration?: any;
formatter?: string;
formattersDirectory?: string;
rulesDirectory?: string | string[];
}

export interface ILinterOptions extends ILinterOptionsRaw {
configuration: any;
formatter: string;
formattersDirectory: string;
rulesDirectory: string | string[];
}
25 changes: 15 additions & 10 deletions src/tslint.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
} from "./configuration";
import {EnableDisableRulesWalker} from "./enableDisableRules";
import {findFormatter} from "./formatterLoader";
import {ILinterOptions, LintResult} from "./lint";
import {ILinterOptionsRaw, ILinterOptions, LintResult} from "./lint";
import {loadRules} from "./ruleLoader";
import {arrayify} from "./utils";

Expand All @@ -44,11 +44,10 @@ class Linter {
private source: string;
private options: ILinterOptions;

constructor(fileName: string, source: string, options: ILinterOptions) {
constructor(fileName: string, source: string, options: ILinterOptionsRaw) {
this.fileName = fileName;
this.source = source;
this.options = options;
this.computeFullOptions();
this.options = this.computeFullOptions(options);
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this! Better to return a new object than mutate

}

public lint(): LintResult {
Expand Down Expand Up @@ -99,13 +98,19 @@ class Linter {
return rules.some((r) => r.equals(rule));
}

private computeFullOptions() {
let {configuration, rulesDirectory} = this.options;
if (configuration == null) {
configuration = DEFAULT_CONFIG;
private computeFullOptions(options: ILinterOptionsRaw = {}): ILinterOptions {
if (typeof options !== "object") {
throw new Error("Unknown Linter options type: " + typeof options);
}
this.options.rulesDirectory = arrayify(rulesDirectory).concat(arrayify(configuration.rulesDirectory));
this.options.configuration = configuration;

let { configuration, formatter, formattersDirectory, rulesDirectory } = options;

return {
configuration: configuration || DEFAULT_CONFIG,
formatter: formatter || "prose",
formattersDirectory: formattersDirectory,
rulesDirectory: arrayify(rulesDirectory).concat(arrayify(configuration.rulesDirectory)),
};
}
}

Expand Down