Skip to content

TrailingComma - false positive for map #745

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
darxriggs opened this issue Aug 28, 2023 · 3 comments
Closed

TrailingComma - false positive for map #745

darxriggs opened this issue Aug 28, 2023 · 3 comments

Comments

@darxriggs
Copy link
Contributor

For this map a violation is generated:

def x = [foo: 1,
         bar: 2]

For this list no violation is generated:

def x = [1,
         2]

I would expect no violation for both maps and lists when the closing square bracket is on the same line as the last entry.

@chrismair
Copy link
Contributor

To be clear , I don't believe the rule documentation (or accompanying tests) makes it clear whether there should be a violation if the last element is followed by the closing square bracket. I would probably assume there should be a violation in that case, since that seems to be more in line with the stated rationale for the rule: "Putting this comma in make is easier to change the order of the elements or add new elements on the end." (ignoring the grammatical errors).

In any case, the inconsistency between lists and maps needs to be addressed.

@darxriggs
Copy link
Contributor Author

If a trailing comma should always be enforced, the rule then should also check that the closing square bracket is not on the same line as the last entry. Maybe also that the opening square bracket is not on the same line as the first entry.

I have seen code like this, just to prevent the CodeNarc violation.

def x = [1,
         2,]

Here the "trailing" comma does not serve the intended purpose.

@chrismair
Copy link
Contributor

Yeah, I don't like that style, either. I think I am inclined to NOT cause a violation for either map or list in that situation, and just make sure it is documented behavior for the rule.

chrismair added a commit that referenced this issue Dec 3, 2023

Verified

This commit was signed with the committer’s verified signature.
ephraimbuddy Ephraim Anierobi
…llowed by closing bracket on the same line for Maps (to be consistent with Lists).
chrismair added a commit that referenced this issue Jan 15, 2024

Verified

This commit was signed with the committer’s verified signature.
ephraimbuddy Ephraim Anierobi
…llowed by closing bracket on the same line for Maps (to be consistent with Lists).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants