Generated errors, warnings and infos

JanSharp commented 3 years ago

Errors

Some function parameter names are invalid identifiers (such as end, function and containing .). In parseParameter() those need to be converted into valid identifiers, most likely by replacing all invalid characters with _ and appending an _ to keywords. Valid identifiers match the regex ^[a-zA-Z_][a-zA-Z0-9_]*$, so basically replace everything that isn't [a-zA-Z0-9_] and prepend with an _ if the first char is a number.

Warnings

All defines are getting the undefined global number assigned to them. Even if that is currently a placeholder, it either needs to be a defined global or local or (imo preferably) just 0.

All the basic types generate undefined-doc-name warnings. Aliases seem to just replace the name you actually want, but we either have to take that compromise or we try out this and see how it goes:

---@class float : number
---@class double : number
---@class int : number
---@class int8 : number
---@class uint : number
---@class uint8 : number
---@class uint16 : number
---@class uint64 : number

(deriving from integer doesn't work because that's apparently just an alias for number but i know it's at least a bit more special than that. just how the class number is more special than just deriving from the class any which doesn't have anything defined. Just language server internals looking weird i guess.)

Infos

If no-implicit-any is enabled:

"Lua.diagnostics.neededFileStatus": {
  "no-implicit-any": "Any",
},

Lots of those warnings get generated. it most likely makes the most sense to disable this completely for all files by adding ---@diagnostic disable: no-implicit-any at the top, though it might be useful for finding issues during development.

The Rest

Once those are all fixed you should be left with 9 errors and 28 warnings (with just lowercase-global warnings disabled in Lua.diagnostics.disable). All of those are luadoc-miss-type-name and undefined-doc-name and looking through them they are all things that also need special treatment.

(How do i know? I hack fixed all of these issues. Initially to see if performance (being diagnostic speed) would be better with all of them fixed and it was noticably better, but still not great.)

Edit 1: you'd probably have a few more warnings than 28, because i have the defines.foo classes defined. Edit 2: oh you'd have way more than that. about 100 or so. In order to reduce warnings for performance testing i did add

---@class CustomDictionary
---@class CustomArray

spiwn commented 3 years ago

Some function parameter names are invalid identifiers

I don't understand. Does the Factorio Api have identifiers that are invalid Lua identifiers? Is the scraper generating such identifiers?

In total I see 2 parameter names that are problematic:

defines.logistic_member_index
...

And two function names that have a trailing whitespace, both named operator. That shouldn't be a problematic identifier, but might need to be handled differently in order to accurately describe the element.

I will have to search for a way to represent those in the Lua Doc (... and the () operator), but they will be fixed. Are those the invalid identifiers you are referring to?

In parseParameter() those need to be converted

As the name suggests that should only parse. In general the whole parsing should try not to modify the data. If some escaping is needed, it should be done by the module for the specific output format, for which they are invalid.

All defines are getting the undefined global number assigned to them

This will be fixed.

All the basic types

It is on the to do list.

should be left with 9 errors and 28 warnings

I don't know. There is a lot of work left, so I have not prioritized cleaning up the problems.

JanSharp commented 3 years ago

As the name suggests that should only parse. In general the whole parsing should try not to modify the data. If some escaping is needed, it should be done by the module for the specific output format, for which they are invalid.

This is true, i didn't think about that and retract my statement.

And two function names that have a trailing whitespace

Yea, it's probably a good idea to make sure everything doesn't have leading or trailing spaces anywhere, i can't think of a case where you'd want them.

Are those the invalid identifiers you are referring to?

Yes and no.

The defines one, yes.
The ... one, yes but that's also special.
The operator one, technically no but yes that's another issue that needs to be handled somehow. i didn't think about those yet.
And the ones i did also mean was actually specifically parameters called end and function and i found them by looking through the problems sumneko.lua was reporting when opening up the generated docs in vscode so you'd most likely find them there yourself. If you don't (which would surprise me) they are:
- LuaGuiElement.select = function(start, end) end
- LuaCommandProcessor.add_command = function(name, help, function) end
- LuaRemote.call = function(interface, function, ...) end

I don't know. There is a lot of work left, so I have not prioritized cleaning up the problems.

That would explain my confusion why you didn't fix some of these problems yet, i knew there was some reason, i just couldn't figure out what it was.

zackgomez commented 3 years ago

I reported a bug in the documentation for the get_logistic_point param name in the factorio forum here: https://forums.factorio.com/viewtopic.php?f=7&t=98000&p=544246#p544246

spiwn commented 3 years ago

Great. It might be better to use https://forums.factorio.com/97880 next time (from the pinned messages in the discord server). (They might tell you that in your thread as well. Sorry if I am just repeating it).

spiwn / FactorioApiScraper