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

isMarkedAsUsed Seems To Be A Phantom Method #4783

Closed
epmatsw opened this Issue Dec 22, 2015 · 7 comments

Comments

Projects
None yet
5 participants
@epmatsw
Copy link

commented Dec 22, 2015

I'm working on a plugin, and in one of my rules, I wanted to check if a specific variable has been used. Fortunately, ESLint provides an API for that, context.isMarkedAsUsed. Unfortunately, using that rule triggers an error:

ror: Cannot read property 'apply' of undefined
    at RuleContext.(anonymous function) [as isMarkedAsUsed] (/Users/wstamper/.nvm/versions/node/v5.1.1/lib/node_modules/eslint/lib/rule-context.js:110:32)
    at /Users/wstamper/github/plugin/rules/check-symbols.js:60:24
    at Array.forEach (native)
    at EventEmitter.Program:exit.lines.forEach (/Users/wstamper/github/plugin/rules/check-symbols.js:56:18)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)
    at NodeEventGenerator.leaveNode (/Users/wstamper/.nvm/versions/node/v5.1.1/lib/node_modules/eslint/lib/util/node-event-generator.js:51:22)
    at CommentEventGenerator.leaveNode (/Users/wstamper/.nvm/versions/node/v5.1.1/lib/node_modules/eslint/lib/util/comment-event-generator.js:111:23)
    at Controller.controller.traverse.leave (/Users/wstamper/.nvm/versions/node/v5.1.1/lib/node_modules/eslint/lib/eslint.js:770:36)
    at Controller.__execute (/Users/wstamper/.nvm/versions/node/v5.1.1/lib/node_modules/eslint/node_modules/estraverse/estraverse.js:397:31)

This happens because in lib/rule-context.js, isMarkedAsUsed is listed as a passthrough function, meaning it should be defined on the eslint object. However, it's not defined there either. The passthrough reference was added in #1911 by 8a13be4, which doesn't appear to have actually ever implemented the function, which would pair nicely with markVariableAsUsed. It seems as though isMarkedAsUsed is a development relic, since it was never documented, unlike the other one. An update to the documentation in b070f1a added a line for isMarkedAsUsed, which got us to our current point.

As it stands, I'm not sure what the correct course of action is. It's possible @glenjamin has an implementation somewhere, since he seems to have written markVariableAsUsed. I'd prefer if we could get a working version of the function in ESLint, since I have a used case for it, but if we can't it probably would be good to remove it from the docs, since it's pretty confusing. I may take a stab at it if we want to go that route, but I'm not confident in my ability to actually implement it.

@eslintbot

This comment has been minimized.

Copy link

commented Dec 22, 2015

@epmatsw Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Dec 22, 2015

@epmatsw

This comment has been minimized.

Copy link
Author

commented Dec 22, 2015

  1. This is in every ESLint version 0.17+.
  2. Used isMarkedAsUsed in a plugin.
  3. See above for stack trace.
  4. Not crash, and behave as documented.
@epmatsw

This comment has been minimized.

Copy link
Author

commented Dec 22, 2015

It's also a slight bummer, but it appears that isMarkedAsUsed was implemented in a WIP in glenjamin@aa7367c. It doesn't look like it actually has the documented signature though, since instead of accepting a name, it take a reference to the node. Which is slightly less useful, but probably more realistic.

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Dec 22, 2015

Some how the docs got updated to include isMarkedAsUsed name but the actual definition didnt make it to the core.
Sorry about that.
What I am seeing right now is that you can check the variable reference for eslintUsed property on it.
Example: https://github.com/eslint/eslint/blob/master/lib/rules/no-unused-vars.js#L146

Now we need to figure out whether to remove the isMarkedAsUsed name from docs or actually implement the function. thoughts @eslint/eslint-team ?

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented Dec 22, 2015

I see no problem adding in implementation for this function.

@glenjamin

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2015

isMarkedAsUsed was present as WIP, but was removed after this comment: #1951 (comment)

Leaving it in the passthrough list, and the subsequent addition of documentation were both in oversights.

@gyandeeps

This comment has been minimized.

Copy link
Member

commented Dec 22, 2015

So based on that comment we need to fix the docs and remove the definition.

@gyandeeps gyandeeps closed this in fe46756 Dec 22, 2015

ilyavolodin added a commit that referenced this issue Dec 22, 2015

Merge pull request #4788 from eslint/issue4783
Fix: Remove `isMarkedAsUsed` function name (fixes #4783)

@eslint eslint bot locked and limited conversation to collaborators Feb 6, 2018

@eslint eslint bot added the archived due to age label Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.