mpeterv / luacheck

A tool for linting and static analysis of Lua code.
MIT License
1.92k stars 322 forks source link

Add full API definition for lua-nginx-module #118

Closed jdesgats closed 7 years ago

jdesgats commented 7 years ago

This is based on the v 0.10.10 API. For clarity the API definition has been moved to a separate file.

mpeterv commented 7 years ago

Thanks, I'll look through this more closely this week and then merge.

Do you know how often are these fields removed in new lua-nginx versions? If only new fields are added, then this new standard remains the superset of all fields in all versions and all is good, but if in some version there were more fields, code against that version will have false positives when checked with this standard.

jdesgats commented 7 years ago

The updates are usually very conservative, from what I can remember since a few years there were very few breaking changes, and none of them removed functions or constants. So I don't think there is any problem on this side.

The only issue that I can see is that we will have to update the API definition whenever new functions will be added in future versions.

mpeterv commented 7 years ago

Some comments after looking through API docs in https://github.com/openresty/lua-nginx-module

(Just in case, when adding commits please don't squash them with already pushed ones so that it's obvious what's updated.)

jdesgats commented 7 years ago

ngx.socket.stream and ngx.worker.exiting appear to be missing in the luacheck definition.

Thanks for pointing this out, I somehow missed these two. Pushed this as a separate commit.

There are modules like ngx.semaphore that have local semaphore = require "ngx.semaphore" as usage example, but does requiring such a module result in it being available as a field of global ngx as well?

No, they are regular Lua module, and don't have any global side effect.

Some string constants (all in ngx.config fields) need to be defined as def_fields("byte", "char", "dump", "find", "format", "gmatch", "gsub", "len", "lower", "match", "rep", "reverse", "sub", "upper") to allow using string methods on them directly, see string_defs table in builtin_standards.lua.

Actually only ngx.config.subsystem is a string: debug is a boolean, prefix and nginx_configure are functions, the versions numbers are integers.

Do you think it would make sense to put some helper functions in a separate module? The def_fields function is already duplicated 3 times, and defining string fields seems like something that other standards might want to do.

mpeterv commented 7 years ago

Do you think it would make sense to put some helper functions in a separate module? The def_fields function is already duplicated 3 times, and defining string fields seems like something that other standards might want to do.

Yes, I'll do that.

Merging, new version will be released this week. Thanks!

jdesgats commented 7 years ago

Great, thanks a lot!