| Summary: | [cross file linting] resolve method calls | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Steve Northover <steve_northover> | ||||||
| Component: | JS Tools | Assignee: | Curtis Windatt <curtis.windatt.public> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | curtis.windatt.public, Olivier_Thomann | ||||||
| Version: | unspecified | ||||||||
| Target Milestone: | 12.0 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows 7 | ||||||||
| Whiteboard: | |||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 489691 | ||||||||
| Attachments: |
|
||||||||
|
Description
Steve Northover
I already have a working prototype of this. I've been tweaking the filtering all last week to try to make it useful but not annoying. There are many cases where Tern does not have defining type information for what you are working on resulting in your file being littered with errors. What happens when you turn it on for the simple example code? Can we have it turned off by default, and people can turn it on? What is the status of this feature? This will be a killer feature for us when/if we can get it working. I'm still not 100% happy with the behaviour and there is no fine grained control over what cases are marked as problems (beyond eslint-disable). Since the rule will be off by default I can still push this into master this week so others can easily try it out. Please do. I will turn the rule on and try it. Created attachment 260166 [details]
WIP
I'm generally happy with the behaviour of this patch, but I found one odd situation due to Tern collecting type information. In markdownEditor.js when you first lint, you get a number of undeclared members for this._stylerAdapter.<foo>. If you run openDeclaration on the member expression <foo> it will find a match defined in the same file. The next time you run linting, we have type information for this._stylerAdapter's properties and don't report any problems.
This is similar to when you visit another file adding its type information to Tern, but happening in a single file.
Have you released the code or just attached the patch? Please don't release for tomorrow unless you are sure the my demo scenario will not break (ie. run it yourself, exactly what I would do, step for step). Thanks. I am unbelievably excited about this feature (as you know). Created attachment 260194 [details]
Tests
Tests for the new rule. Includes 2 skipped tests for functionality we should consider implementing/fixing.
1) If a property is later added to a object, Tern doesn't seem to pick up that property. I think this is the same situation as I saw in markdownEditor where after openDecl Tern then knows about the additional property.
2) If we are seeing a call expression while the type Tern knows about is not a function, we could report a different error ('a' is not a function).
also...
3) I want to experiment with checking all member expressions, not just call expressions. My first attempts at this created far too many errors, but by filtering to only source objects we have at least some properties for, this might be better now.
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=5a636ee89471164c43c87762507b8d9ca7167693 Fix with tests The rule is off by default so it won't affect users. Turn on Undeclared member expression reference: to try it out. I have turned on the setting and will experiment. Curtis, please update the wiki to include this and any other new settings we have added. I am likely seeing the same things as you when running on the demo code. Functions like "getElementById" are not getting resolved despite the fact that I have indicated that I am using the eslint browser environment. I know that these are early days for this feature. (In reply to Steve Northover from comment #10) > I have turned on the setting and will experiment. > > Curtis, please update the wiki to include this and any other new settings we > have added. Wiki was already updated before I committed :) (In reply to Steve Northover from comment #11) > I am likely seeing the same things as you when running on the demo code. > Functions like "getElementById" are not getting resolved despite the fact > that I have indicated that I am using the eslint browser environment. I > know that these are early days for this feature. If running open declaration on getElementID fixes the problem then this is the same issue as me. If not, it is a Tern environment case we don't handle properly. I'll investigate this. (In reply to Steve Northover from comment #11) > I am likely seeing the same things as you when running on the demo code. > Functions like "getElementById" are not getting resolved despite the fact > that I have indicated that I am using the eslint browser environment. I > know that these are early days for this feature. The current rule will not mark errors on any property of document because document doesn't have any known properties (its proto has properties and it has maybeProps). I can improve this by checking the proto, will push this change to master once I have tests for it. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=7a0859f7a5988ea4a089629bec36fecff4fee084 Rule now warns when using an undeclared member on a object with only proto properties (such as exports.Object). Tests have been updated. Marking as FIXED. |