simpleComputed regex edge case

[UnrefinedBrain]

https://github.com/rrd108/vue-mess-detector/blob/eec64e39f611955d38d84879f2439960d5077079/src/rules/vue-strong/simpleComputed.ts#L17

Regex is not able to reliably match the entire code block because it lacks the capability to match unknown numbers of balanced pairs. If there is more than one level of nesting, the current regex here will not match it

Example of issue (regex101 link):

const foo = computed(() => {
  if (bar) {
    if (baz) {
    }
  }
});

highly suggest switching to an AST-based approach rather than regex for this. It might look like this:

const ast = babel.parse(sourceCode); // any ESTree-compatible parser would work
const nodes = esquery.query(ast, 'CallExpression[callee.name="computed"][arguments.0.type="ArrowFunctionExpression"] .arguments:nth-child(1) .body');
for (const node of nodes) {
  const newlines = sourceCode.slice(node.range[0], node.range[1]).match(/\n/g)?.length ?? 0;
  if (newLines > maxNewLineCount) {
    const lineNumber = node.loc.start.line;

    reportProblem(lineNumber, 'too many newlines in computed');
  }
}

[rrd108]

Thanks for the suggestion.

As I try to avoid using a parser, I should check if we really need it here.

[David-Pena]

@rrd108 I played around with this issue (as it comes from Reddit) and when there are multiple conditions inside the computed property it indeed gets weird and the regex start to become more complex.

I didnt try it but something that might work is changing the logic to not use regex either but just find open and close brackets and do the magic (something we have in some rules already like the functionSize and ... probably other one I can't remember rn)

[rrd108]

Yes. That was the one.

[UnrefinedBrain]

As I try to avoid using a parser

I'm curious why this is? Regex is the wrong tool for the job here and a parser/AST is the right tool. I'm not trying to be rude and ofc you can do what you want with your project, I'm just a bit confused why creating maintainability issues, reliability issues, and edge case tradeoffs when you could avoid all of it

[rrd108]

I'm curious why this is? Regex is the wrong tool for the job here and a parser/AST is the right tool. I'm not trying to be

We just missing some benchmarking to check it. If you can do some for us you are very welcome to do so!

[rrd108]

Esprima seems to be a bteer option then babel.

[UnrefinedBrain]

Esprima seems to be a bteer option then babel.

Esprima doesn't support typescript syntax as far as I know. Babel does and it can generate an ESTree-compliant AST that can be used with various libraries like ast-types and esquery

not like SWC which has its own non-ESTree AST and isn't compatible with the myriad of libraries for working with ESTree ASTs

Another option is to use ast-grep API which has its own vue language support

[rrd108]

Esprima doesn't support typescript syntax as far as I know. Babel does and it can generate an ESTree-compliant AST that can be used with various libraries like ast-types and esquery

Yes, I saw that. I created a dedicated issue for this. see #368

[David-Pena]

In this issue the thing is this block should be reported or not? @rrd108 @UnrefinedBrain

const foo = computed(() => {
  if (bar) {
    if (baz) {
    }
  }
});

I did some work with babel parser for the functionSize rule, I wanted to try it out for this rule too but I don't remember what's the issue here

[rrd108]

Depends on what is inside the inner if