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

JavaScript: Missing methods inside modernish JavaScript class #1949

Closed
maxnordlund opened this issue Dec 14, 2018 · 18 comments · Fixed by #1989
Closed

JavaScript: Missing methods inside modernish JavaScript class #1949

maxnordlund opened this issue Dec 14, 2018 · 18 comments · Fixed by #1989
Assignees

Comments

@maxnordlund
Copy link

The name of the parser:
JavaScript (jscript.c)

The command line you used to run ctags:

$ ctags --options=NONE --langmap=javascript:+.mjs -f - models/user.mjs
The content of input file:
import uuid from "uuid"

import * as emails from "../app/emails"
import Controller from "../controllers/controller"
import Group from "./group"
import Model from "./model"
import Order from "./order"

export default class User extends Model {
  static get tableName() {
    return "users"
  }

  static get ["json-schema"]() {
    return {
      type: "object",
      optional: [
        "passwordHash",
        "passwordResetRequestedAt",
        "passwordResetToken",
        "verificationToken",
        "verifiedAt",
      ],
      properties: {
        name: { type: "string" },
        email: { type: "string", format: "email" },
        passwordHash: { type: "string" },
        verificationToken: { type: "string", format: "uuid" },
        verifiedAt: { type: "string", format: "date-time" },
        passwordResetToken: { type: "string", format: "uuid" },
        passwordResetRequestedAt: { type: "string", format: "date-time" },
        address: {
          type: "object",
          properties: {
            streetAddress: { type: "string", description: "The street address. For example, 1600 Amphitheatre Pkwy." },
            postalCode: { type: "string", description: "The postal code. For example, 94043." },
            locality: { type: "string", description: "The locality. For example, Mountain View." },
            region: { type: "string", description: "The region. For example, CA." },
            country: { type: "string", default: "USA", description: "The country. For example, USA. You can also provide the two-letter ISO 3166-1 alpha-2 country code." }
          }
        }
      }
    }
  }

  static get relationMappings() {
    return {
      orders: {
        relation: Model.HasManyRelation,
        modelClass: Order,
        join: {
          from: "users.id",
          to: "orders.user_id"
        }
      },
      groups: {
        relation: Model.ManyToManyRelation,
        modelClass: Group,
        join: {
          from: "users.id",
          through: {
            from: "users_groups.user_id",
            to: "users_groups.group_id"
          },
          to: "groups.id"
        }
      }
    }
  }

  get isAnonymous() { return false }
  get isAuthenticated () { return true }

  async $beforeInsert(queryContext) {
    await super.$beforeInsert(queryContext)
    this.verificationToken = uuid.v4()
  }

  inspect(_depth, _options) {
    return `${this.name} (${this.id}) <${this.email}>`
  }

  async sendVerificationMail() {
    let verificationLink = [
      process.env.HOST,
      Controller.urlFor("account.confirm_page", { token: this.verificationToken })
    ].join("")

    return emails.verification(this.email, { name: this.name, verificationLink })
  }

  async sendResetPasswordMail() {
    let passwordResetLink = [
      process.env.HOST,
      Controller.urlFor("account.password_reset_page", { token: this.passwordResetToken })
    ].join("")

    return emails.passwordReset(this.email, { name: this.name, passwordResetLink })
  }
}

export class AnonymousUser extends User {

  get isAnonymous() { return true }
  get isAuthenticated () { return false }

  sendVerificationMail() {
    throw new TypeError("Can't send a verification email to the anonymous user.")
  }

  sendResetPasswordMail() {
    throw new TypeError("Can't send a password reset email to the anonymous user.")
  }
}

export const ANONYMOUS_USER = new AnonymousUser({ name: "<anonymous>" })

The tags output you are not satisfied with:

AnonymousUser	models/user.mjs	/^export class AnonymousUser extends User {$/;"	c
User	models/user.mjs	/^export default class User extends Model {$/;"	c
passwordResetLink	models/user.mjs	/^    let passwordResetLink = [$/;"	v
sendResetPasswordMail	models/user.mjs	/^  sendResetPasswordMail() {$/;"	m	class:AnonymousUser
sendVerificationMail	models/user.mjs	/^  sendVerificationMail() {$/;"	m	class:AnonymousUser
verificationLink	models/user.mjs	/^    let verificationLink = [$/;"	v

The tags output you expect:

The above, but also tags for the methods/computed properties.

The version of ctags:

$ ctags --version                                                                                                                                                                          11:14:25
Universal Ctags 0.0.0(64f7d61), Copyright (C) 2015 Universal Ctags Team
Universal Ctags is derived from Exuberant Ctags.
Exuberant Ctags 5.8, Copyright (C) 1996-2009 Darren Hiebert
  Compiled: Dec 14 2018, 10:37:50
  URL: https://ctags.io/
  Optional compiled features: +wildcards, +regex, +iconv, +option-directory, +xpath, +case-insensitive-filenames

How do you get ctags binary:

homebrew:

$ brew info universal-ctags/universal-ctags/universal-ctags                                                                                                                                11:58:48
universal-ctags/universal-ctags/universal-ctags: HEAD
Maintained ctags implementation
https://github.com/universal-ctags/ctags
Conflicts with:
  ctags (because this formula installs the same executable as the ctags formula)
/usr/local/Cellar/universal-ctags/HEAD-64f7d61 (325 files, 4.3MB) *
  Built from source on 2018-12-14 at 10:38:07
From: https://github.com/universal-ctags/homebrew-universal-ctags/blob/master/universal-ctags.rb

This is taken from an actual project am working on, but I figured the user class should be common enough to demonstrate my point. For some other module/classes there're even less tags outputted.

I think it's the use of computed properties (get/set), async and ["funky-method"]. The latter is more common then one might think, language level protocols are mostly implemented using well-known symbols.

class InfiniteCounter {
  *[Symbol.iterator]() {
    for (let i = 0;; ++i) yield i
   }
}
@bigfish
Copy link

bigfish commented Jan 23, 2019

Just ran into this as well. Not indexing class methods is quite disappointing...

@bigfish
Copy link

bigfish commented Jan 23, 2019

there is code in the parser to parse ES6 methods.. perhaps my version from homebrew is out of date.

ctags --version output..

Universal Ctags 0.0.0(6aa62c9), Copyright (C) 2015 Universal Ctags Team
Universal Ctags is derived from Exuberant Ctags.
Exuberant Ctags 5.8, Copyright (C) 1996-2009 Darren Hiebert
Compiled: Jun 22 2018, 14:43:13
URL: https://ctags.io/
Optional compiled features: +wildcards, +regex, +iconv, +option-directory, +xpath, +case-insensitive-filenames
~

will try building latest

@bigfish
Copy link

bigfish commented Jan 23, 2019

Still not working on latest version(578ea8b)

@masatake masatake changed the title Missing methods inside modernish JavaScript class JavaScript: Missing methods inside modernish JavaScript class Jan 24, 2019
@kotatsuyaki
Copy link

kotatsuyaki commented Jan 26, 2019

This is happening on one of my projects as well. In my class with get and set functions on top of a class, the getter/setter and rest methods of the class aren't present in the output at all.

@masatake
Copy link
Member

Which standard should I read to understand the requirement?
The input file reported in the initial comment is too large to work on this issue incrementally.

The JavaScript parser doesn't capture latest in the following input:

var obj = {
  log: ['example','test'],
  get latest() {
    if (this.log.length == 0) return undefined;
    return this.log[this.log.length - 1];
  }
}
console.log(obj.latest); // "test".

I think we should introduce getter kind, and make the JavaScript parser capture latest as a getter.
I think we can do the same for setter.

@kotatsuyaki
Copy link

I'm no expert in this so not sure what standard you're referring to, but did you mean this EMCAScript 2015 standard? It's where computed property names, along with set and get were first introduced.

@masatake
Copy link
Member

@AkitakiKou, thank you!
The url you showed helps me.

@masatake
Copy link
Member

I added support for getter and setter in #1989.
Could you try the change?

I guess I have not solved whole the issue yet. Could you lead me the next smaller sub issue like @AkitakiKou did. I have no time to learn JavaScript from scratch.

export default class User extends Model {
  static get tableName() {
    return "users"
  }

  static get ["json-schema"]() {
    return {
      type: "object",
      optional: [
        "passwordHash",
        "passwordResetRequestedAt",
        "passwordResetToken",
        "verificationToken",
        "verifiedAt",
      ]
    }
  }
}

I guess json-schema should be tagged as a getter. Am I correct?

@kotatsuyaki
Copy link

kotatsuyaki commented Jan 27, 2019

Yes, json-schema should be tagged as a getter as well, to my understanding. I'm on mobile so can't try out #1989 , will do when I got time.

@masatake
Copy link
Member

[yamato@slave]~/var/ctags-github% git diff |cat
git diff |cat
diff --git a/parsers/jscript.c b/parsers/jscript.c
index 1a0e9572..064f83c3 100644
--- a/parsers/jscript.c
+++ b/parsers/jscript.c
@@ -1611,6 +1611,7 @@ static bool parseMethods (tokenInfo *const token, const tokenInfo *const class,
 		{
 			bool is_generator = false;
 			bool is_shorthand = false; /* ES6 shorthand syntax */
+			bool is_compusted_propname = false; /* ES6 compusted property name */
 
 			if (isType (token, TOKEN_STAR)) /* shorthand generator */
 			{
@@ -1618,9 +1619,19 @@ static bool parseMethods (tokenInfo *const token, const tokenInfo *const class,
 				readToken (token);
 			}
 
+			if (isType (token, TOKEN_OPEN_SQUARE))
+			{
+				is_compusted_propname = true;
+				readToken (token);
+			}
+
 			copyToken(name, token, true);
 
 			readToken (token);
+
+			if (is_compusted_propname && isType (token, TOKEN_CLOSE_SQUARE))
+				readToken (token);
+
 			is_shorthand = isType (token, TOKEN_OPEN_PAREN);
 			if ( isType (token, TOKEN_COLON) || is_shorthand )
 			{
[yamato@slave]~/var/ctags-github% cat input.js
cat input.js
export default class User extends Model {
  static get tableName() {
    return "users"
  }

  static get ["json-schema"]() {
    return {
      type: "object",
      optional: [
        "passwordHash",
        "passwordResetRequestedAt",
        "passwordResetToken",
        "verificationToken",
        "verifiedAt",
      ]
    }
  }
}
[yamato@slave]~/var/ctags-github% ./ctags -o - input.js
./ctags -o - input.js
User	input.js	/^export default class User extends Model {$/;"	c
json-schema	input.js	/^  static get ["json-schema"]() {$/;"	G	class:User
tableName	input.js	/^  static get tableName() {$/;"	G	class:User

The "json-scheme" is captured well. However, My code assumes that there is only one token betwen [ and ]. This assumption mayl be wrong. @AkitakiKou, or someone, could you let me know the reference document about the computed property name?

@masatake masatake self-assigned this Jan 27, 2019
@bigfish
Copy link

bigfish commented Jan 27, 2019

Hey, great to see some activity on this! getters and setters are great to have -- I will test it out tonight.
Dynamic properties seems like they may be troublesome, as they can be the result of any JS code inside the [] .. if there's a literal string it should be OK, but they why are dynamic properties even used? I wouldn't expend any more effort on this.. I was simply hoping to have regular class methods work.. see here for some docs on ES6 /ES2015 Class syntax, specifically the method syntax.

masatake added a commit to masatake/ctags that referenced this issue Jan 28, 2019
…ted in string literals

Partially close universal-ctags#1949.

Note: in universal-ctags#1949, An idea capturing [Symbol.*] is show.
This change doesn't touch the idea to make the change small.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@masatake
Copy link
Member

@bigfish, thank you.

I pushed the code for capturing computed properties only if thay are represented in a string literal.
I don't touch 'Symbol.*'.

I would like to get your feedback.

@masatake
Copy link
Member

I have worked the get/set keywords and the computed property notations. I will merge the result soon after getting an ack from @b4n.

Do my changes solve this issue?

@bigfish
Copy link

bigfish commented Jan 28, 2019

ok, @masatake I will give it a try.. but not quite sure how to get your latest changes.. is it on a PR, or already in master? I will take a look.

@bigfish
Copy link

bigfish commented Jan 28, 2019

yah, it's working great. Thanks, @masatake!

@masatake
Copy link
Member

Already my changes are in master branch.

@masatake
Copy link
Member

Thank you for testing.

@masatake
Copy link
Member

O.k. I would like to close this. If the changes newly introduced for this issue are not enough, please, open new issues. I expect rather smaller input, URL to standards describing the syntax in the new issues.

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

Successfully merging a pull request may close this issue.

4 participants