Skip to content

Conversation

@BuBuaBu
Copy link

@BuBuaBu BuBuaBu commented Apr 10, 2024

No description provided.

@BuBuaBu BuBuaBu force-pushed the useless_key branch 2 times, most recently from b8857cf to 0654717 Compare April 10, 2024 21:55
Copy link
Member

@alerque alerque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate this and even do personally prefer not quoting keys when not necessary, but hove some reservations about making this a default lint. There are likely to be too many cases in the wild where generated code or in contexts where some keys need protection and some don't for this to be appreciated:

local t = {
   ["a.b"] = true -- good
   ["a'b"] = true -- good
   ["a_b"] = true -- why does this necessarily get called 'bad'?
}

Even normally preferring to use literal keys when possible, this lint would make some normal coding patterns require exceptions.

Have you tried running this on any code bases in the wild that use Luacheck already?

These types of lines are limited using CLI options named ``--[no-]max-string-line-length``, ``--[no-]max-comment-line-length``,
and ``--[no-]max-code-line-length``, with similar config and inline options.

Style issues (6xx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6xx as a header seems to be in conflict with the 7xx code described in the section

:linenos:
local foo = {
["a"] = 0, -- bad
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a "computed property", there is no computation going on here. It is just a quoted value that can be quite handy when Lua can't parse it as a keyword. This is commonly used in generated code when the code generator can't know if the key is going to be an acceptable literal keyword or not.

At the very least calling these "computed" doesn't seem right. And I agree using them is sometimes unnecessary, but I don't think linting them as "bad" is going to be correct all the time.

Style issues (6xx)
-----------------------

Useless computed key (701)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do move ahead with this (I have some reservations about it being on by default) then we'd need to call it "unnecessary", not "useless".

@alerque
Copy link
Member

alerque commented Aug 29, 2024

Gently ping. There are a couple other PRs that are about to drop and I'd like to get a release out soon. No pressure but if this were to make it into the next release, the review comments will need to be dealt with soon. Otherwise it will sit until you (or anybody) gets around to resolving those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants