Comments (12)
It doesn't detect that require
:
detective('// require("xyz")')
// → []
detective('require("xyz")')
// → ["xyz"]
The require
string occuring inside a comment just means detective will parse the file to do the AST walk to check for require calls. If the word require
does not occur in the file at all, detective doesn't even parse it and just returns an empty array immediately. If require
does not occur in some part of the AST, that part is also entirely skipped. This issue is about stripping comments early so we can take advantage of the fast path in more cases.
The issue you're seeing is that a file containing JSX or ES modules is being passed to detective. If the file doesn't contain require
at all, detective will take the fast path (without parsing), and not notice that it uses unsupported syntax. If it does, detective will try to parse it, but it doesn't support JSX so the parsing will fail.
e; see #45 (comment)
from detective.
The require string occuring inside a comment just means detective will parse the file to do the AST walk to check for require calls. If the word require does not occur in the file at all, detective doesn't even parse it and just returns an empty array immediately.
Bingo! That makes sense to me now, thanks :)
I was thinking it was reporting the actual strings.
from detective.
I dig this idea. I took a quick look at strip-comments, and two things came to mind: (1) performance, but maybe it's not so bad; (2) strip-comments doesn't seem maintained anymore, not that it matters for the node-detective use case, but it removes comment-like things from strings (doesn't inspire confidence).
Any other ideas? I'd be awesome to be able to skip files if we can.
from detective.
Not sure about other solutions. We can write our own solution that would do this on per-line iteration basis. Nothing fancy.
from detective.
I must be misunderstanding something; how does Detective report those as requires when it uses the AST? Also, I don't get that when I try it with [email protected]
, which I presume was latest when this issue was created. I get only ["b"]
as the output.
from detective.
@jmm, I think the OP is a bit misworded. The output is the same regardless of version (correctly ["b"]
). detective has this optimization where is checks the source string for something that may look like a require
, before even bothering to parse the AST. If the word "require" doesn't show up in the source, then you can be pretty sure that there isn't a require
call. In the case where a word like "require" shows up in comments (like in jQuery), but not in code, you're parsing the AST for nothing. Stripping comments from source is pretty cheap, so it might be worth doing that before doing the fast check.
from detective.
@zertosh Ah, gotcha, thanks for the explanation. I knew about that test, but I didn't connect the dots because I misinterpreted "parses" in the OP. I was also thrown off by the inclusion of the legit require()
in the example.
Stripping comments from source is pretty cheap, so it might be worth doing that before doing the fast check.
Do you think it'd make sense to do the fast check, then, if that was positive, strip the comments and try it again?
from detective.
This is a pretty bad bug, and is easily encountered:
// eslint-disable-next-line require-jsdoc
function foo() {}
/**
* Does foo.
* @example
* const foo = require('foo')
* foo()
*/
export default function foo() {}
This issue appears to cause documentationjs/documentation#1090.
from detective.
The issue there is that detective doesn't support JSX or ES modules syntax. It should not be run on JSX or ES modules files in the first place.
from detective.
@goto-bus-stop no, the point I'm making in this issue here is that people often use JSDoc @example
comments that contain require
, and // eslint-disable-next-line require-jsdoc
comments which also contains require
.
from detective.
This issue is not about a bug but an optimization. Implementing the optimization would only hide the issue you're describing, not fix it. It would pop back up the moment someone uses a string that contains 'require', or uses a method named someobj.require()
, for example.
from detective.
This issue is not about a bug but an optimization.
This is the repo description:
Find all calls to require() no matter how deeply nested using a proper walk of the AST
If this causing a false positive is not a bug, then I don't know what is…
// eslint-disable-next-line require-jsdoc
There is clearly no call to require()
in that code. I could tell that with simple regex, let alone a “proper walk of the AST”.
I'm happy to be corrected if I'm confused, or misdiagnosed something 😕
from detective.
Related Issues (20)
- Ability to handle cases where require is called with a non-literal parameter HOT 2
- require('something')() HOT 1
- Support array calls HOT 1
- Support AMD syntax for finding dependencies? HOT 1
- 'Missing yield in generator' should not be an error HOT 1
- Add a note about requires with more than string literals HOT 2
- why not use the Regular Expression to find all params in require function?? HOT 2
- Upgrade acorn HOT 1
- Upgrade to use Acorn v2 HOT 3
- Pass `sourceType` option through to acorn HOT 2
- Support ES6 imports HOT 3
- Add support for ES6 template functions HOT 1
- Revert 'require' word optimization HOT 1
- Newer keywords like async/await is not supported HOT 3
- syntax error is returned when object spread is used HOT 1
- Replace `acorn` parser for `babylon`? HOT 5
- Document how `detective` can be extended
- Insecure dependancy "minimist": "^1.1.1" HOT 5
- Update to [email protected]
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from detective.