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

Apply fixes depending on the source file used in the failures #2392

Closed
mgechev opened this issue Mar 23, 2017 · 1 comment · Fixed by #2421 or singapore/lint-condo#272
Closed

Apply fixes depending on the source file used in the failures #2392

mgechev opened this issue Mar 23, 2017 · 1 comment · Fixed by #2421 or singapore/lint-condo#272

Comments

@mgechev
Copy link
Contributor

mgechev commented Mar 23, 2017

Introduction

I'm working on a tool called codelyzer. It exports a few tslint rules which are Angular specific. Internally the tool resolves the templates and styles of Angular components, parses them and performs analysis over them.

Later, based on this analysis the tool produces diagnostics which not always should be reported in the source file which was initially analyzed by tslint, i.e. tslint may initially perform analysis over app.component.ts but the diagnostics can be reported to app.component.html. This happens by providing app.component.html as source file to the template visitor which extends RuleWalker internally.

Actual behavior

Currently the produced replacements which should be applied to templates or styles are applied to the TypeScript file of the component which references them.

Expected behavior

The fixes should be applied to the files where the failure is reported, i.e. if the failure is reported for app.component.html and the value of failure.getFileName() === 'app.component.html' then the fix should be applied to app.component.html.

Fix

The fix will require change in linter.ts, something like:

...
  if (this.options.fix) {
    for (const rule of enabledRules) {
      const ruleFailures = this.applyRule(rule, sourceFile);
      source = fs.readFileSync(fileName, { encoding: "utf-8" });
      const fixsPerFile = this.getFixesPerFile(ruleFailures);
      if (Object.keys(fixsPerFile).length > 0) {
        this.fixes = this.fixes.concat(ruleFailures);
        Object.keys(fixsPerFile)
          .forEach(function (failureFileName) {
            const currentFixes = fixsPerFile[failureFileName];
            const content = fs.readFileSync(failureFileName, { encoding: "utf-8" });
            content = Fix.applyAll(content, currentFixes);
            fs.writeFileSync(failureFileName, content, { encoding: "utf-8" });
            if (fileName === failureFileName) {
              sourceFile = this.getSourceFile(failureFileName, source);
            }
          });
      }
      fileFailures = fileFailures.concat(ruleFailures);
    }
    hasLinterRun = true;
  }
...

private getFixesPerFile(ruleFailures: RuleFailure[]) {
  return ruleFailures
    .reduce((accum, c) => {
      const fn = c.getFileName();
      accum[fn] = accum[fn] || [];
      const fix = c.getFix();
      if (fix) {
        accum[fn].push(fix);
      }
      return accum;
    }, {})
}

Let me know what you think. Would love to open a PR.

The final goal is to provide automatic migration for the deprecations introduced by Angular 4.

The change will not introduce any breaking changes and will not change the current behavior.

mgechev added a commit to mgechev/tslint that referenced this issue Mar 28, 2017
Fix palantir#2392

The spec that I added extends the `Linter` in order to test the extra
method I added. Since tslint doesn't have any rules which apply fixes in
a file different from the one which is currently being linted, I didn't
come up with additional tests.
@mgechev
Copy link
Contributor Author

mgechev commented Mar 28, 2017

Here's the PR #2421.

mgechev added a commit to mgechev/tslint that referenced this issue Mar 29, 2017
Fix palantir#2392

The spec that I added extends the `Linter` in order to test the extra
method I added. Since tslint doesn't have any rules which apply fixes in
a file different from the one which is currently being linted, I didn't
come up with additional tests.
mgechev added a commit to mgechev/tslint that referenced this issue Apr 1, 2017
Fix palantir#2392

The spec that I added extends the `Linter` in order to test the extra
method I added. Since tslint doesn't have any rules which apply fixes in
a file different from the one which is currently being linted, I didn't
come up with additional tests.
nchen63 pushed a commit that referenced this issue Apr 3, 2017
Fix #2392

The spec that I added extends the `Linter` in order to test the extra
method I added. Since tslint doesn't have any rules which apply fixes in
a file different from the one which is currently being linted, I didn't
come up with additional tests.
@adidahiya adidahiya added this to the TSLint v5.1 milestone Apr 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants