SimenB / stylint

Improve your Stylus code with Stylint
https://simenb.github.io/stylint/
GNU General Public License v2.0
348 stars 61 forks source link

Can we use an ast to extend stylint? #303

Open lyt9304 opened 8 years ago

lyt9304 commented 8 years ago

Stylint is using line scan to check the code. But I think Line Scan may be not strong enough for check code. There are many things that we can't decide which one it is just through one line. Using stylus ast could be easier to check some rules than line scan, and will have more accuracy.

Actually I'm working on extension of stylint because my company is using stylint to check our stylus code, and we have many requirements of stylint rules which is not supported yet.

And I use ast for some checking, it is very easy to check expression and definition.

It's very hard to parse or recognize complicated expression through regex, but using ast it will be very easy to gain the result of parsing expression.

Maybe we can use node scan instead of line scan in some rules, although the cost of time will be increasing.

It's just my little opinion for stylint improvment, may be it's not good.

Thank you for your time and opinions!

SimenB commented 8 years ago

I think using AST would be much preferable to regexes. I tried (for like an hour) some months ago to use the core stylus package to parse files, but I didn't get any way with it. Have you had success implementing it?

I suppose the best thing would be if somebody implemented stylus support as a postcss plugin, then you'd be able to use stylelint directly. postcss/postcss#602

lyt9304 commented 8 years ago

stylus exposed parse module, I tried:

    var Parser = require('stylus').Parser
    var parser = new Parser(file.toString())
    try {
      var ast = parser.parse() // get an ast
      ast = JSON.parse(JSON.stringify(ast))
    }catch(err){
      this.msg( 'Stylus parse error!' )
    }

the stylus code

$size = 1
div
  margin - $size
  font-size -(10px - 20px)

generates ast:


{
  "__type": "Root",
  "nodes": [
    {
      "__type": "Ident",
      "name": "$size",
      "val": {
        "__type": "Expression",
        "lineno": 1,
        "column": 9,
        "nodes": [
          {
            "__type": "Unit",
            "val": 1,
            "lineno": 1,
            "column": 9
          }
        ]
      },
      "mixin": false,
      "lineno": 1,
      "column": 9
    },
    {
      "__type": "Group",
      "nodes": [
        {
          "__type": "Selector",
          "inherits": true,
          "segments": [
            {
              "__type": "Literal",
              "val": "div",
              "string": "div",
              "prefixed": false,
              "lineno": 2,
              "column": 1
            }
          ],
          "optional": false,
          "lineno": 2,
          "column": 1
        }
      ],
      "block": {
        "__type": "Block",
        "scope": true,
        "lineno": 2,
        "column": 1,
        "nodes": [
          {
            "__type": "Property",
            "segments": [
              {
                "__type": "Ident",
                "name": "margin",
                "val": {
                  "__type": "Null"
                },
                "mixin": false,
                "lineno": 3,
                "column": 3
              }
            ],
            "lineno": 3,
            "column": 3,
            "expr": {
              "__type": "Expression",
              "lineno": 3,
              "column": 12,
              "nodes": [
                {
                  "__type": "UnaryOp",
                  "op": "-",
                  "expr": {
                    "__type": "Ident",
                    "name": "$size",
                    "val": {
                      "__type": "Null"
                    },
                    "mixin": false,
                    "lineno": 3,
                    "column": 12
                  },
                  "lineno": 3,
                  "column": 12
                }
              ]
            }
          },
          {
            "__type": "Property",
            "segments": [
              {
                "__type": "Ident",
                "name": "font-size",
                "val": {
                  "__type": "Null"
                },
                "mixin": false,
                "lineno": 4,
                "column": 3
              }
            ],
            "lineno": 4,
            "column": 3,
            "expr": {
              "__type": "Expression",
              "lineno": 4,
              "column": 26,
              "nodes": [
                {
                  "__type": "UnaryOp",
                  "op": "-",
                  "expr": {
                    "__type": "Expression",
                    "lineno": 4,
                    "column": 22,
                    "nodes": [
                      {
                        "__type": "BinOp",
                        "left": {
                          "__type": "Unit",
                          "val": 10,
                          "type": "px",
                          "lineno": 4,
                          "column": 15
                        },
                        "right": {
                          "__type": "Unit",
                          "val": 20,
                          "type": "px",
                          "lineno": 4,
                          "column": 22
                        },
                        "op": "-",
                        "lineno": 4,
                        "column": 22
                      }
                    ]
                  },
                  "lineno": 4,
                  "column": 26
                }
              ]
            }
          }
        ]
      },
      "lineno": 1,
      "column": 10
    }
  ]
}

Then I use BFS/DFS to traversal the tree, and doing some check with node unit.

rossPatton commented 8 years ago

this would be much preferable to regex. when I started this project I wasn't aware of AST, looking back I would have gone that route instead

i would prefer the postcss route - it seems like things are going in that direction

it might even make more sense to just have a preset for tools like https://github.com/stylelint/stylelint (which I find to be very lacking, but it is an AST powered extendable linter, so we could make it more useful for stylus users)

PhiLhoSoft commented 8 years ago

CSSLint and Stylelint are merging... https://github.com/CSSLint/csslint/issues/668 Would be nice to add stylint to the effort! :wink:

SimenB commented 8 years ago

Great that they're merging, not sure it matters much to this effort though.