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

Introduce autofix API #254

Merged
merged 1 commit into from
Jun 13, 2023
Merged

Introduce autofix API #254

merged 1 commit into from
Jun 13, 2023

Conversation

wata727
Copy link
Member

@wata727 wata727 commented May 2, 2023

See also terraform-linters/tflint#266

This PR adds a new Runner API for autofix. The newly added EmitIssueWithFix API allows plugins to not only emit issues but also automatically apply fixes for the issues.

Below is an example of autofixing the instance type "t2.micro" to "t3.micro".

runner.EmitIssueWithFix(
	rule,
	"Use t3 family instead of t2 family",
	expr.Range(),
	func (f tflint.Fixer) error {
		return f.ReplaceText(expr.Range(), `"t3.micro"`)
	}
)

Unlike EmitIssue API, this function allows you to pass a function callback for autofix in the 4th argument. Plugin developers can use the tflint.Fixer in the function callback to change their source code.

The following APIs are available in tflint.Fixer:

  • ReplaceText(hcl.Range, ...(string|tflint.TextNode))
  • InsertTextBefore(hcl.Range, string)
  • InsertTextAfter(hcl.Range, string)
  • Remove(hcl.Range)
  • RemoveAttribute(*hcl.Attribute)
  • RemoveBlock(*hcl.Block)
  • RemoveExtBlock(*hclext.Block)

They are inspired by ESLint's autofix APIs. See the GoDoc for these details.

Also, it doesn't change the source code directly, but there are some APIs available to help with that:

  • TextAt(hcl.Range) tflint.TextNode
  • ValueText(cty.Value) string
  • RangeTo(to string, filename string, start hcl.Pos) hcl.Range

Please refer to terraform-linters/tflint#266 for how to use the autofix implemented by plugins from TFLint.


Next, describe the internal design. The sequence diagram for communication between the host and plugins is shown below:

sequenceDiagram
  participant RuleSet as RuleSet (plugin)
  participant Rule as Rule (plugin)
  participant TFLint as TFLint (host)
  RuleSet->>Rule: Start rule.Check()
  Rule->>TFLint: EmitIssue()
  TFLint->>Rule: applied?
  Rule->>Rule: fixer.ReplaceText()
  Rule->>RuleSet: End rule.Check()
  RuleSet->>TFLint: ApplyChanges()
Loading

This PR adds the ApplyChanges API and changes the EmitIssue API to return whether the issue has been applied (e.g. it may be ignored by tflint-ignore annotations).

After emitting an issue with EmitIssue API, if it is not ignored by an annotation, invoke the given fix function. tflint.Fixer has all the source code internally and the fix function rewrites that as a byte sequence.

After the rule inspection is finished, the rewritten source code is applied to the host server by the ApplyChanges API. From now on, the body obtained by APIs such as GetModuleContent will be the fixed source code.

There are two implementation considerations worth mentioning here. One is not to use hclwrite to rewrite the source code. The hclwrite provides APIs for rewriting HCL, but since it handles AST with a different abstraction level than hclsyntax, it is not possible to rewrite the corresponding expression from hcl.Expression. In order to realize easy rewriting using hcl.Expression or hcl.Range, I thought that it was necessary to implement the rewriting operation as a byte string instead of hclwrite. This implementation is in internal.Fixer.

The other is byte shift by rewriting. The source code is rewritten using the byte index pointed to by Byte field in hcl.Pos. However, if multiple rewrite operations occur, the original hcl.Pos may have a byte index that does not reflect the result of the previous rewrite operation. To solve this problem, the internal.Fixer tracks byte index shifts that occur during rewriting. The shift is reflected in the byte indices of subsequent operations, allowing the original range to be rewritten appropriately.

The discussion on which these designs were based can be found in terraform-linters/tflint#266 (comment).

@wata727 wata727 force-pushed the introduce_auto_fixer branch from f4e30ce to d07f142 Compare May 2, 2023 08:48
@wata727 wata727 force-pushed the introduce_auto_fixer branch 10 times, most recently from 8a5cb28 to 4f23c16 Compare May 6, 2023 15:44
@wata727 wata727 force-pushed the introduce_auto_fixer branch 6 times, most recently from 2d0aa75 to 8b2ba55 Compare May 20, 2023 15:25
@wata727 wata727 force-pushed the introduce_auto_fixer branch 4 times, most recently from 98b8c10 to e764c34 Compare May 27, 2023 09:00
@wata727 wata727 force-pushed the introduce_auto_fixer branch 7 times, most recently from 7187ca9 to 060104c Compare June 3, 2023 09:24
@wata727 wata727 force-pushed the introduce_auto_fixer branch 2 times, most recently from 656766c to 4a39bf6 Compare June 4, 2023 16:55
@wata727 wata727 force-pushed the introduce_auto_fixer branch from 4a39bf6 to 64de942 Compare June 5, 2023 17:05
@wata727 wata727 marked this pull request as ready for review June 7, 2023 17:25
@wata727 wata727 merged commit b875e92 into master Jun 13, 2023
@wata727 wata727 deleted the introduce_auto_fixer branch June 13, 2023 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants