universal-ctags / ctags

A maintained ctags implementation
https://ctags.io
GNU General Public License v2.0
6.38k stars 618 forks source link

TypeScript function with ternary (or if-statement) short-circuits parser #4000

Open hovsater opened 1 month ago

hovsater commented 1 month ago

The name of the parser: TypeScript

The command line you used to run ctags:

$  ctags --options=NONE test.ts

The content of input file:

const foo = () => {
  return 1 < 2 ? 'foo' : 'bar'
}

const bar = () => {}

The tags output you are not satisfied with:

foo test.ts /^const foo = () => {$/;"   C

The tags output you expect:

bar test.ts /^const bar = () => {}$/;"  C
foo test.ts /^const foo = () => {$/;"   C

The version of ctags:

$ ctags --version
Universal Ctags 6.1.0, Copyright (C) 2015-2023 Universal Ctags Team
Universal Ctags is derived from Exuberant Ctags.
Exuberant Ctags 5.8, Copyright (C) 1996-2009 Darren Hiebert
  Compiled: May 10 2024, 16:30:18
  URL: https://ctags.io/
  Output version: 0.0
  Optional compiled features: +wildcards, +regex, +gnulib_fnmatch, +gnulib_regex, +iconv, +option-directory, +xpath, +json, +interactive, +yaml, +case-insensitive-filenames, +packcc, +optscript, +pcre2

How do you get ctags binary:

$ brew info universal-ctags
==> universal-ctags: stable p6.1.20240512.0 (bottled), HEAD
Maintained ctags implementation
https://github.com/universal-ctags/ctags
Conflicts with:
  ctags (because this formula installs the same executable as the ctags formula)
Installed
/opt/homebrew/Cellar/universal-ctags/p6.1.20240512.0 (44 files, 3.7MB) *
  Poured from bottle using the formulae.brew.sh API on 2024-05-12 at 09:47:00
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/u/universal-ctags.rb
License: GPL-2.0-only
==> Dependencies
Build: autoconf, automake, docutils, pkg-config, python@3.12
Required: jansson, libyaml, pcre2
==> Options
--HEAD
    Install HEAD version
==> Analytics
install: 3,111 (30 days), 9,759 (90 days), 37,618 (365 days)
install-on-request: 2,494 (30 days), 7,769 (90 days), 29,131 (365 days)
build-error: 0 (30 days)
b4n commented 1 month ago

It's actually not the ternary but the < which is incorrectly recognized as the start of a template and "eats" the rest of the file (or until a ; if you add one at the end of the line).

b4n commented 1 month ago

FWIW this ugly hack works:

diff --git a/parsers/typescript.c b/parsers/typescript.c
index 0e04b1b69..063388dfa 100644
--- a/parsers/typescript.c
+++ b/parsers/typescript.c
@@ -1173,7 +1173,7 @@ static void parseVariable (bool constVar, bool localVar, const int scope, tokenI
    {
        clearPoolToken (token);
        parsed = tryInSequence (token, false,
-                               parseTemplate,
+                               nestLevel > 0 ? parseComment : parseTemplate,
                                parseComment,
                                parseStringRegex,
                                parseStringSQuote,
char101 commented 1 month ago

parseFunctionBody also need to be modified to handle case like this

class A {
  constructor() {
    if (1 > 2 && 2 < 1) {
    }
  }

  function1() {
    [].forEach(v => 0);
  }
}

It seems to work by removing parseTemplate function parseFunctionBody, although I'm not sure of the consequence of removing it.

diff --git a/parsers/typescript.c b/parsers/typescript.c
index 0e04b1b69..19ca1b663 100644
--- a/parsers/typescript.c
+++ b/parsers/typescript.c
@@ -1173,7 +1173,7 @@ static void parseVariable (bool constVar, bool localVar, const int scope, tokenI
        {
                clearPoolToken (token);
                parsed = tryInSequence (token, false,
-                                                               parseTemplate,
+                                                               nestLevel > 0 ? parseComment : parseTemplate,
                                                                parseComment,
                                                                parseStringRegex,
                                                                parseStringSQuote,
@@ -1396,7 +1396,6 @@ static void parseFunctionBody (const int scope, tokenInfo *const token)
                                                                parseStringDQuote,
                                                                parseStringTemplate,
                                                                parseStringRegex,
-                                                               parseTemplate,
                                                                NULL);

        } while (parsed && ! isType (token, TOKEN_OPEN_CURLY));
@@ -1414,7 +1413,6 @@ static void parseFunctionBody (const int scope, tokenInfo *const token)
                                                                parseStringDQuote,
                                                                parseStringTemplate,
                                                                parseStringRegex,
-                                                               parseTemplate,
                                                                parseVarKeyword,
                                                                parseLetKeyword,
                                                                parseConstKeyword,
masatake commented 1 month ago

If a parser maintainer is absent, what we can believe are our valuable test cases. For Typescript, we have enough!

image

Fell free to make a pull request though I can say merge it in soon.

If none try, I will add this to the end of my queue.

ksamborski commented 1 month ago

yeah, so parseTemplate is needed in parseFunctionBody to handle other cases. I don't have time now to look into this but as @masatake said, if you'd like to fix this please run tests to see if everything works.

char101 commented 1 month ago

Another issue: d is recognized as method

class A {
  f() {
    const a = b => {
      if (!c) {}
    }
    d.e()
  }
}
A       test2.ts        /^class A {$/;" c
a       test2.ts        /^    const a = b => {$/;"      C       method:A.f
d       test2.ts        /^    d.e()$/;" m       class:A
f       test2.ts        /^  f() {$/;"   m       class:A

But not when ! is removed from !c

class A {
  f() {
    const a = b => {
      if (c) {}
    }
    d.e()
  }
}
A       test2.ts        /^class A {$/;" c
a       test2.ts        /^    const a = b => {$/;"      C       method:A.f
f       test2.ts        /^  f() {$/;"   m       class:A

AST tree: https://ts-ast-viewer.com/#code/MYGwhgzhAECC0G8BQ1rAPYDsIBcBOArsDungBQCUiAvitAGaWJ2obY7RjQC80ARjwB8zVKOgBLetDIBCYFQS0xS0QBMAdAFNKdWrSA