ldez / tagliatelle

A linter that handles struct tags.
Apache License 2.0
47 stars 9 forks source link

feat: add support for struct level overrides #18

Closed geoah closed 1 year ago

geoah commented 1 year ago

Hey! :)

We were looking into using this with @pd93 to lint some stuff, but unfortunatelly we have a bunch of snowflake structs that need to be different to everything else.

This is a proposal to add a struct-level comment that can override the rules for the given struct. Happy to change the comment format if there are better suggestions.

The way this is implemented is not really ideal but it should be good enough for this usecase without complicating things even more. It basically adds astCommentGroup to the node filter, and attempts to keep track of the rules defined before each struct, and clearing them once the struct or another comment has been processed.

The idea is taken from the way go/doc deals with a similar situation and some discussion on edge cases can be found here. As we only care and deal with structs, and this comment only applies on structs, I can't think there will be an issue with going forward with this implementation, but any comments on possible issues would be more than welcome.

Let me know if this is something you think would be of interest and what you think it would take to get this merged! :D

Regards, ~geoah

cc @pd93

ldez commented 1 year ago

Hello,

I will define some concepts before talking about the feature. In Go, there are comments but there is a specific kind of comment called directive. The directives follow a specific format //^[a-z]+:[a-z]+. The directives are excluded from godoc.

Then you should use directives, not comments.

Now, we can talk about the feature itself.

In a real context, the format of a JSON field (for example) should be consistent inside a package (ex: an API client). A directive seems not the best approach: the granularity (I mean by struct) forces you to set this on all the structs.

For me, the best approach is to be able to define rules by package (#3)

I am open to discuss, maybe I missed something.

geoah commented 1 year ago

Hey @ldez,

Separating the rules by package is something that would indeed make sense. Had considered the package approach as well, but my concern was that it might make debugging the lint error harder as it pushes the declaration of the rule further away from the struct and that it makes the feature less flexible.

I do agree though with you that it does make sense to keep the whole package consistent, and if you have a reason to have structs with different type of tags it makes sense that you keep them in separate packages.

You mentioned on #3 that you're working on this but it's been a while since that comment. Is that still the case? If this is something you're happy for someone else to pickup or help with do let me know.

I'll be closing this PR as it seems to not be needed.

Thank you very much for your time! :) ~ geoah

ps. Don't think I've ever noticed that there is a difference between directives and comments, thanks for pointing that out.