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

Lint documentation examples #2271

Closed
btmills opened this Issue Apr 9, 2015 · 20 comments

Comments

Projects
None yet
6 participants
@btmills
Member

btmills commented Apr 9, 2015

Split from #2257 (comment).

With file loaders, we can now lint the example code in documentation files. At the very least, this should catch syntax errors. I think it would be cool if we used configuration comments and actually applied the rule to the valid examples. (Not sure how we would only check the invalid ones for syntax; that'll take some thought.)

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Aug 12, 2015

Member

I'll take a crack at this.

Member

IanVS commented Aug 12, 2015

I'll take a crack at this.

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Aug 27, 2015

Member

Here are a list of the current rules and status:

  • accessor-pairs
  • array-bracket-spacing
  • arrow-parens
  • arrow-spacing
  • block-scoped-var
  • block-spacing
  • brace-style
  • callback-return
  • camelcase
  • comma-dangle
  • comma-spacing
  • comma-style
  • complexity
  • computed-property-spacing
  • consistent-return
  • consistent-this
  • constructor-super
  • curly
  • default-case
  • dot-location
  • dot-notation
  • eol-last
  • eqeqeq
  • func-names
  • func-style
  • generator-star-spacing
  • guard-for-in
  • handle-callback-err
  • id-length
  • id-match
  • indent
  • init-declarations
  • key-spacing
  • linebreak-style
  • lines-around-comment
  • max-depth
  • max-len
  • max-nested-callbacks
  • max-params
  • max-statements
  • new-cap
  • newline-after-var
  • new-parens
  • no-alert
  • no-array-constructor
  • no-bitwise
  • no-caller
  • no-catch-shadow
  • no-class-assign
  • no-cond-assign
  • no-console
  • no-constant-condition
  • no-const-assign
  • no-continue
  • no-control-regex
  • no-debugger
  • no-delete-var
  • no-div-regex
  • no-dupe-args
  • no-dupe-class-members
  • no-dupe-keys
  • no-duplicate-case
  • no-else-return
  • no-empty
  • no-empty-character-class
  • no-empty-label
  • no-eq-null
  • no-eval
  • no-ex-assign
  • no-extend-native
  • no-extra-bind
  • no-extra-boolean-cast
  • no-extra-parens
  • no-extra-semi
  • no-fallthrough
  • no-floating-decimal
  • no-func-assign
  • no-implicit-coercion
  • no-implied-eval
  • no-inline-comments
  • no-inner-declarations
  • no-invalid-regexp
  • no-invalid-this
  • no-irregular-whitespace
  • no-iterator
  • no-labels
  • no-label-var
  • no-lone-blocks
  • no-lonely-if
  • no-loop-func
  • no-mixed-requires
  • no-mixed-spaces-and-tabs
  • no-multiple-empty-lines
  • no-multi-spaces
  • no-multi-str
  • no-native-reassign
  • no-negated-in-lhs
  • no-nested-ternary
  • no-new
  • no-new-func
  • no-new-object
  • no-new-require
  • no-new-wrappers
  • no-obj-calls
  • no-octal
  • no-octal-escape
  • no-param-reassign
  • no-path-concat
  • no-plusplus
  • no-process-env
  • no-process-exit
  • no-proto
  • no-redeclare
  • no-regex-spaces
  • no-restricted-modules
  • no-return-assign
  • no-script-url
  • no-self-compare
  • no-sequences
  • no-shadow
  • no-shadow-restricted-names
  • no-spaced-func
  • no-sparse-arrays
  • no-sync
  • no-ternary
  • no-this-before-super
  • no-throw-literal
  • no-trailing-spaces
  • no-undef
  • no-undefined
  • no-undef-init
  • no-underscore-dangle
  • no-unexpected-multiline
  • no-unneeded-ternary
  • no-unreachable
  • no-unused-expressions
  • no-unused-vars
  • no-use-before-define
  • no-useless-call
  • no-var
  • no-void
  • no-warning-comments
  • no-with
  • object-curly-spacing
  • object-shorthand
  • one-var
  • operator-assignment
  • operator-linebreak
  • padded-blocks
  • prefer-arrow-callback
  • prefer-const
  • prefer-reflect
  • prefer-spread
  • prefer-template
  • quote-props
  • quotes
  • radix
  • require-jsdoc
  • require-yield
  • semi
  • semi-spacing
  • sort-vars
  • space-after-keywords
  • space-before-blocks
  • space-before-function-paren
  • spaced-comment
  • space-infix-ops
  • space-in-parens
  • space-return-throw-case
  • space-unary-ops
  • strict
  • use-isnan
  • valid-jsdoc
  • valid-typeof
  • vars-on-top
  • wrap-iife
  • wrap-regex
  • yoda
  • global-require

Rules waiting for DavidAnson/markdownlint#5:

  • indent
  • no-trailing-spaces

Additionally:

  • no-void purposely includes a syntaxError.
  • no-warning-comments cannot be set in a config comment with "location": "anywhere" (#3619)
Member

IanVS commented Aug 27, 2015

Here are a list of the current rules and status:

  • accessor-pairs
  • array-bracket-spacing
  • arrow-parens
  • arrow-spacing
  • block-scoped-var
  • block-spacing
  • brace-style
  • callback-return
  • camelcase
  • comma-dangle
  • comma-spacing
  • comma-style
  • complexity
  • computed-property-spacing
  • consistent-return
  • consistent-this
  • constructor-super
  • curly
  • default-case
  • dot-location
  • dot-notation
  • eol-last
  • eqeqeq
  • func-names
  • func-style
  • generator-star-spacing
  • guard-for-in
  • handle-callback-err
  • id-length
  • id-match
  • indent
  • init-declarations
  • key-spacing
  • linebreak-style
  • lines-around-comment
  • max-depth
  • max-len
  • max-nested-callbacks
  • max-params
  • max-statements
  • new-cap
  • newline-after-var
  • new-parens
  • no-alert
  • no-array-constructor
  • no-bitwise
  • no-caller
  • no-catch-shadow
  • no-class-assign
  • no-cond-assign
  • no-console
  • no-constant-condition
  • no-const-assign
  • no-continue
  • no-control-regex
  • no-debugger
  • no-delete-var
  • no-div-regex
  • no-dupe-args
  • no-dupe-class-members
  • no-dupe-keys
  • no-duplicate-case
  • no-else-return
  • no-empty
  • no-empty-character-class
  • no-empty-label
  • no-eq-null
  • no-eval
  • no-ex-assign
  • no-extend-native
  • no-extra-bind
  • no-extra-boolean-cast
  • no-extra-parens
  • no-extra-semi
  • no-fallthrough
  • no-floating-decimal
  • no-func-assign
  • no-implicit-coercion
  • no-implied-eval
  • no-inline-comments
  • no-inner-declarations
  • no-invalid-regexp
  • no-invalid-this
  • no-irregular-whitespace
  • no-iterator
  • no-labels
  • no-label-var
  • no-lone-blocks
  • no-lonely-if
  • no-loop-func
  • no-mixed-requires
  • no-mixed-spaces-and-tabs
  • no-multiple-empty-lines
  • no-multi-spaces
  • no-multi-str
  • no-native-reassign
  • no-negated-in-lhs
  • no-nested-ternary
  • no-new
  • no-new-func
  • no-new-object
  • no-new-require
  • no-new-wrappers
  • no-obj-calls
  • no-octal
  • no-octal-escape
  • no-param-reassign
  • no-path-concat
  • no-plusplus
  • no-process-env
  • no-process-exit
  • no-proto
  • no-redeclare
  • no-regex-spaces
  • no-restricted-modules
  • no-return-assign
  • no-script-url
  • no-self-compare
  • no-sequences
  • no-shadow
  • no-shadow-restricted-names
  • no-spaced-func
  • no-sparse-arrays
  • no-sync
  • no-ternary
  • no-this-before-super
  • no-throw-literal
  • no-trailing-spaces
  • no-undef
  • no-undefined
  • no-undef-init
  • no-underscore-dangle
  • no-unexpected-multiline
  • no-unneeded-ternary
  • no-unreachable
  • no-unused-expressions
  • no-unused-vars
  • no-use-before-define
  • no-useless-call
  • no-var
  • no-void
  • no-warning-comments
  • no-with
  • object-curly-spacing
  • object-shorthand
  • one-var
  • operator-assignment
  • operator-linebreak
  • padded-blocks
  • prefer-arrow-callback
  • prefer-const
  • prefer-reflect
  • prefer-spread
  • prefer-template
  • quote-props
  • quotes
  • radix
  • require-jsdoc
  • require-yield
  • semi
  • semi-spacing
  • sort-vars
  • space-after-keywords
  • space-before-blocks
  • space-before-function-paren
  • spaced-comment
  • space-infix-ops
  • space-in-parens
  • space-return-throw-case
  • space-unary-ops
  • strict
  • use-isnan
  • valid-jsdoc
  • valid-typeof
  • vars-on-top
  • wrap-iife
  • wrap-regex
  • yoda
  • global-require

Rules waiting for DavidAnson/markdownlint#5:

  • indent
  • no-trailing-spaces

Additionally:

  • no-void purposely includes a syntaxError.
  • no-warning-comments cannot be set in a config comment with "location": "anywhere" (#3619)

IanVS added a commit that referenced this issue Aug 28, 2015

IanVS added a commit that referenced this issue Aug 28, 2015

ilyavolodin added a commit that referenced this issue Aug 28, 2015

Merge pull request #3487 from eslint/fix_2271
Docs: Add linting to rule doc example code blocks (1 of 3) (refs #2271)
@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Aug 28, 2015

Member

Wowza.

Member

nzakas commented Aug 28, 2015

Wowza.

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Aug 29, 2015

Member

💫 💤

Member

IanVS commented Aug 29, 2015

💫 💤

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Aug 30, 2015

Member

Second PR in progress: #3574

Member

IanVS commented Aug 30, 2015

Second PR in progress: #3574

@IanVS IanVS self-assigned this Sep 1, 2015

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Sep 2, 2015

Member

The way the markdown linter plugin currently works, all expected errors must be provided on the line for which they are reported. This causes some readability problems when there are multiple errors for the same line. After eslint/eslint-plugin-markdown#9 is solved, it will be possible to shorten this to simply the number of expected errors.

Tracking the list of rules with more than one error on a line so they can be cleaned up later:

  • array-bracket-spacing
  • object-curly-spacing
  • quote-props
  • semi-spacing
Member

IanVS commented Sep 2, 2015

The way the markdown linter plugin currently works, all expected errors must be provided on the line for which they are reported. This causes some readability problems when there are multiple errors for the same line. After eslint/eslint-plugin-markdown#9 is solved, it will be possible to shorten this to simply the number of expected errors.

Tracking the list of rules with more than one error on a line so they can be cleaned up later:

  • array-bracket-spacing
  • object-curly-spacing
  • quote-props
  • semi-spacing
@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Sep 6, 2015

Member

I've finally hit a rule for which I cannot provide expected errors the way I've been doing. That is valid-jsdoc, because as soon as I add an /*error ...*/ comment on the line with the error, the comment is no longer valid jsdoc and stops being considered by the rule. For now, I'll set the rule to warning remove the info tag, but will likely need a better long-term solution.

Member

IanVS commented Sep 6, 2015

I've finally hit a rule for which I cannot provide expected errors the way I've been doing. That is valid-jsdoc, because as soon as I add an /*error ...*/ comment on the line with the error, the comment is no longer valid jsdoc and stops being considered by the rule. For now, I'll set the rule to warning remove the info tag, but will likely need a better long-term solution.

IanVS added a commit that referenced this issue Sep 6, 2015

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Sep 6, 2015

Member

How about using line comments instead of block comments?

Member

nzakas commented Sep 6, 2015

How about using line comments instead of block comments?

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Sep 7, 2015

Member

How about using line comments instead of block comments?

That works as long as there's only one error. Or once eslint/eslint-plugin-markdown#9 is implemented, this could be just the ticket.

Member

IanVS commented Sep 7, 2015

How about using line comments instead of block comments?

That works as long as there's only one error. Or once eslint/eslint-plugin-markdown#9 is implemented, this could be just the ticket.

IanVS added a commit that referenced this issue Sep 7, 2015

ilyavolodin added a commit that referenced this issue Sep 8, 2015

Merge pull request #3574 from eslint/issue2271
Docs: Add linting to rule doc example code blocks (2 of 3) (refs #2271)
@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Sep 8, 2015

Member

(broken out from main tracking comment above)

Some rule examples require ecmaFeatures: { modules: true } and others require modules: false. Until a better solution can be found (like functionality for modules similar to #3314), I'm removing the info string from the rules which require modules to be turned off. These are:

  • no-delete-var
  • no-dupe-args
  • no-lone-blocks (a solution to #2134 would also work here)
  • no-octal-escape
  • no-octal
  • no-restricted-syntax
  • no-sequences
  • no-shadow-restricted-names
  • no-with
  • prefer-reflect
  • global
Member

IanVS commented Sep 8, 2015

(broken out from main tracking comment above)

Some rule examples require ecmaFeatures: { modules: true } and others require modules: false. Until a better solution can be found (like functionality for modules similar to #3314), I'm removing the info string from the rules which require modules to be turned off. These are:

  • no-delete-var
  • no-dupe-args
  • no-lone-blocks (a solution to #2134 would also work here)
  • no-octal-escape
  • no-octal
  • no-restricted-syntax
  • no-sequences
  • no-shadow-restricted-names
  • no-with
  • prefer-reflect
  • global
@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Sep 20, 2015

Member

These are the rules mentioned above which have issues:

  • indent (DavidAnson/markdownlint#5)
  • no-trailing-spaces (DavidAnson/markdownlint#5)
  • no-void (purposeful syntax error, no way to mark as expected)
  • no-warning-comments (#3619 - fixed)
  • valid-jsdoc (eslint/eslint-plugin-markdown#9 - fixed)
  • no-delete-var (turn off modules/strict mode)
  • no-dupe-args (turn off modules/strict mode)
  • no-octal-escape (turn off modules/strict mode)
  • no-octal (turn off modules/strict mode)
  • no-restricted-syntax (turn off modules/strict mode)
  • no-sequences (turn off modules/strict mode)
  • no-shadow-restricted-names (turn off modules/strict mode)
  • no-with (turn off modules/strict mode)
  • prefer-reflect (turn off modules/strict mode)

A few rules require ecmaFeatures: modules, and will not be fully lintable until #3698 is solved.

  • id-length
  • no-multi-spaces
  • object-curly-spacing

Also needs #3698 (or eslint/eslint-plugin-markdown#19):

  • jsx-quotes

Other cleanup issues:

  • block-scoped-var (additional linting error)
  • global-require (recently added, no config comments)
  • indent (change in indent results)
  • no-lone-blocks (error line moved)
  • no-unused-expressions (rule change?)
Member

IanVS commented Sep 20, 2015

These are the rules mentioned above which have issues:

  • indent (DavidAnson/markdownlint#5)
  • no-trailing-spaces (DavidAnson/markdownlint#5)
  • no-void (purposeful syntax error, no way to mark as expected)
  • no-warning-comments (#3619 - fixed)
  • valid-jsdoc (eslint/eslint-plugin-markdown#9 - fixed)
  • no-delete-var (turn off modules/strict mode)
  • no-dupe-args (turn off modules/strict mode)
  • no-octal-escape (turn off modules/strict mode)
  • no-octal (turn off modules/strict mode)
  • no-restricted-syntax (turn off modules/strict mode)
  • no-sequences (turn off modules/strict mode)
  • no-shadow-restricted-names (turn off modules/strict mode)
  • no-with (turn off modules/strict mode)
  • prefer-reflect (turn off modules/strict mode)

A few rules require ecmaFeatures: modules, and will not be fully lintable until #3698 is solved.

  • id-length
  • no-multi-spaces
  • object-curly-spacing

Also needs #3698 (or eslint/eslint-plugin-markdown#19):

  • jsx-quotes

Other cleanup issues:

  • block-scoped-var (additional linting error)
  • global-require (recently added, no config comments)
  • indent (change in indent results)
  • no-lone-blocks (error line moved)
  • no-unused-expressions (rule change?)

IanVS added a commit that referenced this issue Sep 21, 2015

Docs: Prepare for rule doc linting (refs #2271)
Squashed Commits:
* 4f9db2a - Remove js info string from blocks requiring modules
* edc6edf - Specify '/*eslint-env es6*/' as appropriate
* d3372e6 - Return js info string to rules which require non-strict mode
* df83b86 - Cleanup failing rule examples

IanVS added a commit that referenced this issue Sep 21, 2015

ilyavolodin added a commit that referenced this issue Sep 25, 2015

Merge pull request #3866 from eslint/issue2271
Docs: Prepare for rule doc linting (refs #2271)
@gyandeeps

This comment has been minimized.

Show comment
Hide comment
@gyandeeps

gyandeeps Jan 19, 2016

Member

@IanVS Can we close this based on #4104?

Member

gyandeeps commented Jan 19, 2016

@IanVS Can we close this based on #4104?

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Jan 19, 2016

Member

It would still be nice to find a way to lint the rule examples. Some discussion has been happening in eslint/eslint-plugin-markdown#46 as to how we might accomplish this.

Member

IanVS commented Jan 19, 2016

It would still be nice to find a way to lint the rule examples. Some discussion has been happening in eslint/eslint-plugin-markdown#46 as to how we might accomplish this.

@ilyavolodin

This comment has been minimized.

Show comment
Hide comment
@ilyavolodin

ilyavolodin Jan 22, 2016

Member

I have what might be somewhat of a radical idea, but what if we stop adding examples to documentation and instead, generate them from test cases for rules? We can provide placeholders for where they are supposed to go into. This way we know that example are always going to be valid.

Member

ilyavolodin commented Jan 22, 2016

I have what might be somewhat of a radical idea, but what if we stop adding examples to documentation and instead, generate them from test cases for rules? We can provide placeholders for where they are supposed to go into. This way we know that example are always going to be valid.

@platinumazure

This comment has been minimized.

Show comment
Hide comment
@platinumazure

platinumazure Jan 22, 2016

Member

@ilyavolodin Awesome idea, 👍 from me, but sometimes there are a lot of test cases. How do you propose avoiding information overload?

Member

platinumazure commented Jan 22, 2016

@ilyavolodin Awesome idea, 👍 from me, but sometimes there are a lot of test cases. How do you propose avoiding information overload?

@btmills

This comment has been minimized.

Show comment
Hide comment
@btmills

btmills Jan 23, 2016

Member

There's probably a way to cross-reference specific tests...

The following patterns are not considered problems:

[](#eslint-test some-valid-example)
[](#eslint-test this-one-too)

The following patterns are considered problems:

[](#eslint-test first-bad-example)
[](#eslint-test another-bad-example)
var testCases = module.exports = {
    valid: {
        "anonymousTest();",
        { code: "namedTest();", name: "some-valid-example" }
    },
    invalid: {
        { /* ... */ name: "another-bad-example" }
    }
};

ruleTester.run("rule-name", rule, testCases);

It would require refactoring the tests to export the test cases, but I suppose there could be a step during site generation that could extract those.

Member

btmills commented Jan 23, 2016

There's probably a way to cross-reference specific tests...

The following patterns are not considered problems:

[](#eslint-test some-valid-example)
[](#eslint-test this-one-too)

The following patterns are considered problems:

[](#eslint-test first-bad-example)
[](#eslint-test another-bad-example)
var testCases = module.exports = {
    valid: {
        "anonymousTest();",
        { code: "namedTest();", name: "some-valid-example" }
    },
    invalid: {
        { /* ... */ name: "another-bad-example" }
    }
};

ruleTester.run("rule-name", rule, testCases);

It would require refactoring the tests to export the test cases, but I suppose there could be a step during site generation that could extract those.

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Apr 21, 2016

Member

@btmills @IanVS any updates on this? Still planning to move forward?

Member

nzakas commented Apr 21, 2016

@btmills @IanVS any updates on this? Still planning to move forward?

@IanVS

This comment has been minimized.

Show comment
Hide comment
@IanVS

IanVS Apr 21, 2016

Member

I have no immediate plans to pursue this myself, no.

Member

IanVS commented Apr 21, 2016

I have no immediate plans to pursue this myself, no.

@btmills

This comment has been minimized.

Show comment
Hide comment
@btmills

btmills Apr 21, 2016

Member

There's an open issue (eslint/eslint-plugin-markdown#46) discussing how to support expected errors in linter examples. How about closing this, tracking possible ideas over there, and if we can figure that out, we can open a new issue here?

Member

btmills commented Apr 21, 2016

There's an open issue (eslint/eslint-plugin-markdown#46) discussing how to support expected errors in linter examples. How about closing this, tracking possible ideas over there, and if we can figure that out, we can open a new issue here?

@nzakas

This comment has been minimized.

Show comment
Hide comment
@nzakas

nzakas Apr 21, 2016

Member

Sounds good.

Member

nzakas commented Apr 21, 2016

Sounds good.

@nzakas nzakas closed this Apr 21, 2016

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.