NightrainsRbx / RobloxLsp

Roblox Luau Language Server based on Lua by sumneko.
https://devforum.roblox.com/t/roblox-lsp-full-intellisense-for-roblox-and-luau/717745
MIT License
215 stars 52 forks source link

Undefined member behaviour inconsistent #4

Closed chess123mate closed 4 years ago

chess123mate commented 4 years ago

Referencing the undefined member warning in the devforum.roblox.com post:

When writing the same name twice or more across all the scripts, it will automatically ignored.

There are two problem cases (that I believe are reasonably common):

  1. The name of the instance you are referring to has changed (and you referred to it more than once) or you made a typo and copy/pasted that typo into another script. In this case, no warning will result, despite it being an issue that needs fixing.
  2. The name is correct, but the project hasn't imported an ignore list. If the name is only used once, a warning will occur.

Suggestions:

NightrainsRbx commented 4 years ago

The way it is implemented right now it's a problem, and needs to be changed.

Although most people will not have a .datamodel.json as it is very impractical to use and has to be constantly updated, it will be useful when it can be updated automatically by a Roblox Studio plugin that communicates with the Language Server.

So your suggestions are not the best way to fix this problem, the only thing I can do at the moment is to disable this feature, or make it optional.

chess123mate commented 4 years ago

I don't understand your conclusion.

The only beneficial thing this warning can do for you is to tell you when you've misspelled something or forgotten to update the name of something. However, if you change the name of something in the place and forget to update it in the scripts, this warning won't trigger if 2+ scripts reference the original name. Doesn't this defeat the main benefit?

I think the best version of this feature (that could be implemented now as far as I know):

If it worked like that, I wouldn't have needed to disable this feature for my projects (as I don't have datamodel.json set up), as it would be rare for it to give a false positive.

All this said, I imagine that it's a ton of effort to implement for minimal benefit, so I understand if you choose to disable the feature. (I wouldn't mind it being disabled, since if I want to get someone set up to use your plugin, disabling this warning would just be another step I'd need to communicate.)

NightrainsRbx commented 4 years ago

Hi, just to be clear, I'm not going to disable the undefined member warning, just ignore names that were used twice or more.

And it's because as you said, if you use the name once, you will still get a warning, but if you use the name more times but misspelled, you will not.