Lint for literal values used in expressions

[zoechi]

literal values (String, int, double, not sure about others) should only be assigned to named constants (except 0, 1)

[pq]

Could you add some good/bad examples? How would you phrase the advice?

[zoechi]

Prefer creating a named constant or an enum instead of using a literal value (magic number) in an expression directly.

class Person {
  String name;
  int age;
  List<String> validate() {
    var errors = [];
    if(name == null || name.isEmpty) {
      errors.add('"name" is mandatory.');
    } else if(name.length > 32) {
      errors.add('The maximum name length is 32');
    }

    if(age == null) {
      errors.add('"age" is mandatory.');
    } else if(age < 18) {
      errors.add('The minimum age is 18 to register for this service.');
    }
    return errors;
  }
}

should be something like

class Person {
  static const maxNameLength = 32;
  static const minAge = 18;
  static const maxNameLengthExceededMessage = 'The maximum name length is $maxNameLength';
  static const minAgeBelowMinAgeMessage = 'The minimum age is $minAge to register for this service.';
  static const nameMandatoryMessage = '"name" is mandatory.';
  static const ageMandatoryMessage = '"age" is mandatory.';

  String name;
  int age;

  List<String> validate() {
    var errors = [];
    if (name == null || name.isEmpty) {
      errors.add(nameMandatoryMessage);
    } else if (name.length > maxNameLength) {
      errors.add(maxNameLengthExceededMessage);
    }

    if (age == null) {
      errors.add(ageMandatoryMessage);
    } else if (age < minAge) {
      errors.add(minAgeBelowMinAgeMessage);
    }
    return errors;
  }
}

[a14n]

I'm not really tempted by this lint. Especially when the literal is used only once. This makes readability harder IMHO because you end up jumping to variable declarations all the time to really understand what is going on.

Remember that one of the most difficult task in software development is to find (variable|class|member)'s names ;)

[zoechi]

I don't think this should be forced on users.
The configuration story is not settled as far as I understand but I hope

• the lints derived from the style guide will be default with a way to opt out globally or per library, class, function/method, or expression
• additional lints like this one should be opt-in on package level using a config file, with the ability to specify exceptions as listed for default lints above.

This would for example allow to do a quality check with this specific lint enabled where each magic number/string can be checked if it is still ok to keep the literal or create a constant instead.
After that check it can be disabled for the package again or disabled on the code lines where exceptions are accepted.

[a14n]

Ok, thanks for the clarification. I thought it was a general lint that would have been activated by default. Sorry for the noise ;)

[pq]

@a14n : no apologies necessary. Input is a good thing! And configurability is hot topic #57153 (and #57189).

@zoechi : thanks for the clarification. It looks like you're asking for a flavor of "magic number" checking.

For reference, checkstyle's MagicNumber check is defined like this:

Checks that there are no "magic numbers", where a magic number is a numeric literal that is not defined as a constant. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

[pq]

Further rationale for a check of this sort over on this stack overflow thread.

[zoechi]

Not sure it's a flavor. I would say it's exactly that ;-)

[amrgetment]

updated link for magic number
https://checkstyle.sourceforge.io/checks/coding/magicnumber.html#MagicNumber

For reference, checkstyle's MagicNumber check is defined like this:

[amrgetment]

Any updates regarding this lint?

[srawlins]

No updates.