blizzy78 / varnamelen

Go analyzer checking that the length of a variable's name matches its usage scope
MIT License
20 stars 2 forks source link

Ignore `ok` stomping without ignoring `ok` entirely #33

Open devnev opened 1 month ago

devnev commented 1 month ago

Related to #1

I have a function with a series of lookups and casts as it traverses untyped JSON while reporting errors, i.e.

fooval, ok := root["foo"]
if !ok {
  return errors.New("missing key foo")
}
fooobj, ok := fooval.(map[string]any)
if !ok {
  return errors.New(".foo must be an object")
}
// etc...

Nicer than ignoring type assertions or the ok name in general would be for varnamelen to handle the stomping - that could even apply to the err name - as a value assigned to ok or err should probably be used in the next few lines and not afterwards.

blizzy78 commented 1 month ago

I'm not sure I understand what the issue is. You can use the -maxDistance parameter to specify the maximum distance that is considered "short." Short variable names used over a short distance are ignored. Does that not work for you?

devnev commented 1 month ago

Not quite, let me see if I can explain it differently.

Its mainly about variable names being re-used. In particular, ok is getting re-used a lot over enough distance that it isn't really "short" - but each use is a new assignment followed by a check of the new value. So a given value assigned to ok is used immediately, and only lives for a "short" time, making the name ok acceptable. But the variable ok seems to live a long time (more than I could reasonable set for -maxDistance) because it keeps getting a new value written into it. That long lifetime gets flagged by varnamelen. Maybe ok is special enough to just exclude it entirely, like err, but I'm wondering if there isn't a better alternative by considering a write to be a new variable.