Skip to content

Feature: Built-in linter engine - #2066

Merged
Timothee Guerin (timotheeguerin) merged 27 commits into
microsoft:mainfrom
timotheeguerin:feature/linter-engine
Jun 26, 2023
Merged

Feature: Built-in linter engine#2066
Timothee Guerin (timotheeguerin) merged 27 commits into
microsoft:mainfrom
timotheeguerin:feature/linter-engine

Conversation

@timotheeguerin

@timotheeguerin Timothee Guerin (timotheeguerin) commented Jun 14, 2023

Copy link
Copy Markdown
Member

Fix #1996

Implementation for the linter engine

  • Rule declaration
  • Rulesets
  • New entry in the tspconfig.yaml
  • Docs
  • Tests

@github-actions

Copy link
Copy Markdown
Contributor

Changes in this PR will be published to the following url to try(check status of TypeSpec Pull Request Try It pipeline for publish status):
Playground: https://cadlplayground.z22.web.core.windows.net/prs/2066/

Website: https://cadlwebsite.z1.web.core.windows.net/prs/2066/

Comment thread packages/best-practices/test/rules/casing.rule.test.ts Outdated
Comment thread docs/extending-typespec/linters.md Outdated
});
beforeEach(() => {
const runner = createTestRunner();
ruleTester = createRuleTester(runner, requiredDocRule, "@typespec/my-linter");

@timotheeguerin Timothee Guerin (timotheeguerin) Jun 14, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This differ from the linter library because now as the emitted diagnostic is fully configured within the rule there is no ref of the library anymore. (While before it was just reporting a std diagnostic).

This makes it that if we want the test to be able to check the resolved diagnostic we need to explicitly pass the library name.

There is a few options here:

  1. Have what we have here, a little more extra config
  2. Remove that parameter(or make it optional) and then the diagnostic in the test will not be prefixed by the lbirary name. Maybe just have a placeholder code: "{library-name}:no-foo-model",
  3. The toEmitDiagnostics actually doesn't expect the resolve diagnostic but just what the rule pass so you'd have that below
ruleTester.expect(`model Foo {}`).toEmitDiagnostics({
    messageId: "default"
});

Disadantage of this is we don't get to test message interpolation
For reference that's what eslint does but Im not sure they have the same interpolation system as us.

Comment thread packages/compiler/src/core/program.ts Outdated
Comment thread packages/compiler/src/core/linter.ts

@daviwil David Wilson (daviwil) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!

Comment thread packages/compiler/src/core/types.ts Outdated
@timotheeguerin
Timothee Guerin (timotheeguerin) merged commit 62eb0c5 into microsoft:main Jun 26, 2023
@timotheeguerin
Timothee Guerin (timotheeguerin) deleted the feature/linter-engine branch June 26, 2023 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linter system implementation

3 participants