[no-unsafe-optional-property-assignment] too noisy

[danielnixon]

The new no-unsafe-optional-property-assignment rule works, and it flags unsafe assignment as described in the original ticket https://github.com/danielnixon/eslint-plugin-total-functions/issues/39 but it's probably too noisy in practice to add to the recommended rule set.

Before we add it to the recommended rule set, we should see if we can relax it in any (safe) ways. Some ideas to pursue:

1. scope analysis
2. don't flag an assignment as unsafe when the RHS is an object literal. We will need to tread carefully here to avoid false negatives. If the object literal contains an object spread operator ... then we still have to flag it. If the object literal contains references to other values, we still have to flag it. It's only if the object literal is itself made up of other literals exclusively that this would be safe

[danielnixon]

This also applies to the new no-unsafe-mutable-readonly-assignment for the same reasons. Need to consider how to avoid flagging object (and array) literals there too.

[iliubinskii]

Hi, thank you for the plugin.

I think these should be considered as false positives:

interface A {
  readonly x?: string;
}

const a: A = {};

const b: RegExpExecArray[] = [];

At first glance, the fix looks easy. You just need to check that VariableDeclarator contains ObjectExpression or ArrayExpression.

This can be added as an option "allowExpressions":

{
  "total-functions/no-unsafe-optional-property-assignment": [
    "warn", 
    {
      allowExpressions: true
    }
  ]
}

Even if expressions can be unsafe, it is better to disable expressions with an option than to disable entire rule because it is not configurable.

[danielnixon]

@ilyub good point. I'm happy to accept a PR for that change. Object and array literals currently trigger no-unsafe-mutable-readonly-assignment as well. Relaxing both rules to not flag object literals sounds good to me.

Just be careful of the case where the object or array literal has properties (or entries) that are not themselves literals. If any property or entry is a reference to some other value, the rule will still have to flag those instances.

[danielnixon]

I'm deprecating this rule so closing this issue.