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

linter: add deprecation code check, linter declarations #212

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

flakey5
Copy link
Member

@flakey5 flakey5 commented Mar 17, 2025

Adds a check to the linter that ensures all of the deprecation codes in doc/api/deprecations.md are in numerical order.

This also required adding linting declarations to the linter (i.e. <!-- md-lint ... -->), since some of the missing deprecations are ignored for whatever reason.

TODO:

  • write tests for the check & declarations
  • deprecation type check
  • cleanup

@flakey5 flakey5 requested a review from a team as a code owner March 17, 2025 22:38
@flakey5 flakey5 marked this pull request as draft March 17, 2025 22:38
@flakey5
Copy link
Member Author

flakey5 commented Mar 17, 2025

cc @ovflowd @araujogui

* @returns {import('./types').LintIssue[]}
*/
const lint = entry => {
const lint = (entry, declarations) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

The declarations kinda feel retrofitted in but not sure if there's a better way to pass them

const [name, ...value] = declaration.split(' ');

switch (name) {
case 'skip-deprecation': {
Copy link
Member Author

Choose a reason for hiding this comment

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

Wdyt of defining these definitions similarly to the rules & reporters? (i.e. have them defined in a declarations/skip-deprecation.mjs, then here we look them up & call them if they exist)

let expectedCode = 1;

for (const deprecation of deprecations || []) {
while (declarations.skipDeprecation.includes(expectedCode)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

probably a nicer way to do this loop

@AugustinMauroy
Copy link
Member

This test fail because it's use this fixture

Maybe use snapshot instead of fixture

if you use snapshot you ill need:

    "test": "node --test --experimental-test-snapshots",
    "test:coverage": "node --experimental-test-coverage --test",
    "test:update-snapshots": "node --test --experimental-test-snapshots --test-update-snapshots",
    "test:watch": "node --test --experimental-test-snapshots --watch",

and test will look like:

it('should call each rule with the provided entry', (t) => {
    const rule1 = mock.fn(() => []);
    const rule2 = mock.fn(() => []);

    const engine = createLinterEngine([rule1, rule2]);

    engine.lint(assertEntry);

    assert.strictEqual(rule1.mock.callCount(), 1);
    assert.strictEqual(rule2.mock.callCount(), 1);

    t.assert.snapshot(rule1.mock.calls[0].arguments);
    t.assert.snapshot(rule2.mock.calls[0].arguments);
  });

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.

2 participants