remarkjs / remark-lint

plugins to check (lint) markdown code style
https://remark.js.org
MIT License
944 stars 129 forks source link

Fixable rules #82

Open sindresorhus opened 8 years ago

sindresorhus commented 8 years ago

I'm aware I could use Remark to modify the Markdown, but I'm looking for a way to add autofixing to remark-lint rules, similar to ESLint.

See how ESLint solves it: http://eslint.org/docs/developer-guide/working-with-rules#applying-fixes

Could be a --fix flag to remark-lint CLI.

wooorm commented 8 years ago

That would be really awesome! And it’s a pretty smart idea, to do it like that. Not entirely sure about al the methods though, why not just start/end position, and new text, so more like:

function finalNewline(ast, file) {
  var contents = file.toString();
  var last = contents.length - 1;
  var message;

  if (last > -1 && contents.charAt(last) !== '\n') {
    message = file.warn('Missing newline character at end of file');
    message.fix = [last, last, '\n']; // start offset, end offset, replacement text
  }
}

From what I gather, ESLint/xo allows running with --fix to apply all fixes. Does it also have a prompt base interface where users are asked whether to apply each fix? Something like:

> remark -u lint --fix-promt

Yields:

- asdasdasd
+ asdasdasd\n
Missing newline character at end of file
Want to apply this fix? (yN)
sindresorhus commented 8 years ago

Not entirely sure about al the methods though, why not just start/end position, and new text, so more like:

The ESLint fixer works on AST, not offsets. This one should do the same.

From what I gather, ESLint/xo allows running with --fix to apply all fixes.

It tries to apply as many fixes as possible:

Note that the fix is not immediately applied and may not be applied at all if there are conflicts with other fixes. If the fix cannot be applied, then the problem message is reported as usual; if the fix can be applied, then the problem message is not reported. - http://eslint.org/docs/developer-guide/working-with-rules#applying-fixes

Does it also have a prompt base interface where users are asked whether to apply each fix?

No. The goal should be to just fix everything without user interaction. I guess it could prompt for ambiguous cases.

I would strongly recommend digging into more deeply how ESLint does it. Discussions in the issue tracker, etc. They've spent a lot of time on it.

wooorm commented 8 years ago

The ESLint fixer works on AST, not offsets. This one should do the same.

Not really, the fix() function returns a {range: [index, index], text: text} (through those helper functions), you can also see them in the json formatter.

No. The goal should be to just fix everything without user interaction. I guess it could prompt for ambiguous cases.

That works really well for some of the rules, like inserting some white-space or a few characters, but not so much for bigger changes (as ESLint mentions in there docs), as they often interfere with other changes.

Plus it’d be cool for fenced-code-flag to try to detect the code language and suggest that, or alternatively use something the user types in!

dirkroorda commented 6 years ago

I do not use —fix because of the existence of prettier. It seems prettier is also underway for markdown, using the remark-parser, but it currently lacks sufficient output options. It does do word wrap for long lines. Currently, I use first prettier and then remark for fixing my markdown. It works on the command line, and from within vim, with a single keypress ( the ALE plugin). So I think fixing based on individual rules might be a waste of time. But you do need a fair amount of options that control how to format an AST.

muescha commented 4 years ago

is there a way to use the eslint ecosystem:

ChristianMurphy commented 4 years ago

is there a way to use the eslint ecosystem

Maybe, can you clarify what you mean?

  • exporting remark-lint messages with fixes and suggestions in to a json
  • transforming it to a eslint format
  • and then run the es-lint fixer?

Again, maybe. Can you explain the value add of this? The rule syntax of unified currently doesn't have suggested fixes exported. Either way this would need to be implemented. It sounds like this is suggesting implementing a fixer, then disabling it, and exporting to ESLint?

ChristianMurphy commented 4 years ago

Another approach for autofixing, using an attacher with linting context, and a transformer to update the document, similar to Stylelint's approach. https://stylelint.io/developer-guide/rules#add-autofix

JounQin commented 4 years ago

@muescha Actually eslint-mdx is doing this.

muescha commented 4 years ago

@JounQin i did not found any docs about it... can you point me to it?

JounQin commented 4 years ago

@muescha I've pointed the doc link. https://github.com/mdx-js/eslint-mdx#mdxremark

mdx/remark

Integration with remark-lint plugins, it will read remark's configuration automatically via cosmiconfig. But .remarkignore will not be respected, you should use .eslintignore instead.

muescha commented 4 years ago

possible fixable depends on your remark plugins:

🤔 i did not expected it behind this sentence ...

coderaiser commented 2 years ago

Would be amazing to have ability to determine that rule should make some modifications to markdown file or just to report. For example 🐊 Putout rule has a split to fix, report and traverse:

module.exports.report = () => 'Avoid debugger statement';

module.exports.fix = (path, {options}) => {
    path.remove();
};

module.exports.traverse = ({push}) => ({
    'debugger'(path) {
        push(path);
    },
});

So when you pass --fix it will call the method fix and remove the node, otherwise it will call report. Right now I have two remark-lint rules:

remove-dependency-status-badge.md:

import {lintRule} from 'unified-lint-rule';

export const removeDependenciesStatusBadge = lintRule('remark-lint:remove-dependencies-status-badge', (tree, file) => {
    const children = tree.children.filter(isDependencyStatus(file));
    tree.children = children;

    const [heading] = children;

    if (heading.type !== 'heading')
        return;

    const headingChildren = heading.children.filter(isDependencyLink(file));
    tree.children[0].children = headingChildren;
});

const isDependencyStatus = (file) => (child) => {
    if (child.type !== 'definition')
        return true;

    if (child.label === 'DependencyStatusURL') {
        file.message('Remove DependencyStatusURL', child);
        return false;
    }

    if (child.label === 'DependencyStatusIMGURL') {
        file.message('Remove DependencyStatusIMGURL', child);
        return false;
    }

    return true;
};

const isDependencyLink = (file) => (child) => {
    if (child.type !== 'linkReference')
        return true;

    if (child.children[0].label === 'DependencyStatusIMGURL') {
        file.message('Remove reference to DependencyStatusIMGURL', child);
        return false;
    }

    return true;
};

And remove-trailing-whitespaces-from-heading:

import {lintRule} from 'unified-lint-rule';

export const removeTrailingWhitespacesFromHeading = lintRule('remark-lint:remove-trailing-whitespaces-from-heading', (tree, file, options) => {
    console.log(options);
    const [heading] = tree.children;

    if (heading.type !== 'heading')
        return;

    const latest = heading.children[heading.children.length - 1];

    if (latest.type === 'text' && / $/.test(latest.value)) {
        latest.value = latest.value.slice(0, -1);
        report(file, latest);
    }

    tree.children[0].children = heading.children;
});

function report(file, child) {
    file.message('Avoid trailing whitespaces', child);
}

I'm using remark-lint in processor-markdown and when I run 🐊 Putout without --fix flag I have information about trailing whitespaces and dependency status badge. But I need only information about dependency badge. But AST modified and everything is reported at once.

Actually there is a possibility to pass fix flag in rule options (this is not quite right, but possible). But I'm using preset according to create custom rule tutorial, and cannot pass any options from the outside.

@wooorm could you please suggest me a way to handle this case and not modify AST in report mode and only do changes in fix mode?

coderaiser commented 2 years ago

Here is what I came up with. Create one runner rule:

import {lintRule} from 'unified-lint-rule';
import removeDependenciesStatusBadge from './remove-dependencies-status-badge.mjs';
import removeTrailingWhitespacesFromHeading from './remove-trailing-whitespaces-from-heading.mjs';

const plugins = [
    removeDependenciesStatusBadge,
    removeTrailingWhitespacesFromHeading,
];

const maybeEmptyArray = (a) => isArray(a) ? a : [];

const {isArray} = Array;

export const run = lintRule('remark-lint:run', (tree, file, options) => {
    for (const {fix, traverse, report, name} of plugins) {
        const nodes = traverse(tree);

        for (const node of maybeEmptyArray(nodes)) {
            if (options.fix) {
                fix(node);
                continue;
            }

            const message = report(node);
            file.message(`${name}: ${message}`, node);
        }
    }
});

export const rules = {
    plugins,
};

And here is how my rules looks like:

remove-dependencies-status-badge:

const report = () => 'Remove dependencies status badge';

export default {
    name: 'remove-dependencies-status-badge',
    traverse,
    fix,
    report,
};

function fix(nodes, tree) {
    const children = tree.children.filter(isDependencyStatus(nodes));
    tree.children = children;

    const [heading] = children;

    if (heading.type !== 'heading')
        return;

    const headingChildren = heading.children.filter(isDependencyLink);
    tree.children[0].children = headingChildren;
}

function traverse(tree) {
    const nodes = [];
    tree.children.filter(isDependencyStatus(nodes));
    const [first] = nodes;

    return [first];
}

const isDependencyStatus = (nodes) => (child) => {
    if (child.type !== 'definition')
        return true;

    if (child.label === 'DependencyStatusURL') {
        nodes.push(child);
        return false;
    }

    if (child.label === 'DependencyStatusIMGURL') {
        nodes.push(child);
        return false;
    }

    return true;
};

const isDependencyLink = (child) => {
    if (child.type !== 'linkReference')
        return true;

    if (child.children[0].label === 'DependencyStatusIMGURL')
        return false;

    return true;
};

And remove-trailing-whitespaces-from-heading:

const report = () => 'Avoid trailing whitespaces';

const fix = (heading, tree) => {
    const latest = heading.children[heading.children.length - 1];

    if (latest.type === 'text' && / $/.test(latest.value))
        latest.value = latest.value.slice(0, -1);

    tree.children[0].children = heading.children;
};

const traverse = (tree) => {
    const [heading] = tree.children;

    if (heading.type !== 'heading')
        return;

    const latest = heading.children[heading.children.length - 1];

    if (latest.type === 'text' && / $/.test(latest.value))
        return [latest];
};

export default {
    name: 'remove-trailing-whitespaces-from-heading',
    fix,
    traverse,
    report,
};

And everything works good. But! This format is incompatible with format used by remark-lint, and I need to determine is it fix-format-rule with fix in options, or it's remark-lint rule.

By the way similar format looks amazing for ESLint, here is example. It's much shorter and simpler then most ESLint rules.