Skip to content
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

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

Open
hovsater opened this issue May 14, 2024 · 6 comments
Open
Assignees

Comments

@hovsater
Copy link

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
Copy link
Member

b4n commented May 14, 2024

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
Copy link
Member

b4n commented May 14, 2024

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
Copy link

char101 commented May 16, 2024

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
Copy link
Member

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
Copy link
Member

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
Copy link

char101 commented May 17, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants