detects string constant property reference as a computed property

[turadg]

To repro:

  export const PRICE_LOCK_PERIOD = 'PriceLockPeriod';
  const priceLockPeriod = params[PRICE_LOCK_PERIOD].value;

The second line errors with message,

arbitrary computed property names are not allowed in Jessie; use leading '+'

The proposed remedy is also wrong: +PRICE_LOCK_PERIOD is NaN

Fixing this thoroughly will require type information.

[erights]

The Jessie rule is correct. The Jessie + rule is there to allow computed numeric lookup, but for brevity without loss of meaningful safety, we accept that + also allows any stringified number including the strings
NaN, Infinity, and -Infinity as well as fractions and negative numbers.

The safety achieved by this rule is that the computed property is guaranteed not to overlap with any plainly named string property, especially those that are inherited from Object.prototype or Array.prototype. It also prevents using symbol-named properties.

Our agoric-sdk Keyword rules are motivated by overlapping safety concerns. Ideally, we could extend Jessie with a rule to also allow computed property access that was guaranteed to be either a number-name or a Keyword. But I don't know of an adequately terse way to do that.

Fixing this thoroughly will require type information.

Only if the type information is guaranteed sound. I doubt this could be achieved starting from TypeScript types.

For Jessie conformance, the proper fix here is to change to

const priceLockPeriod = params.PriceLockPeriod.value;

which is reliably statically analyzable. The computed property syntax only adds value when the specific property name is not statically known and unique at the call site.

cc @dtribble for a contrary opinion ;)

[erights]

For allowing computed keyword-named property lookup in Jessie, probably the most practical immediate path is to introduce a helper function

export const getSomething = (record, keyword) => {
  assertKeywordName(keyword);
  return record[keyword];
};
harden(getSomething);

But with a name that is both good and very short. I don't currently have any suggestion.

[erights]

The helper function implementation itself violates Jessie rules, and so should exempt itself. But it could then be safely used from Jessie code.

[turadg]

The safety achieved by this rule is that the computed property is guaranteed not to overlap with any plainly named string property, especially those that are inherited from Object.prototype or Array.prototype. It also prevents using symbol-named properties.

Are you saying PRICE_LOCK_PERIOD is a computed property? It's a constant. The proposed remedy params.PriceLockPeriod.value is equivalent to the sample code, is it not?

Only if the type information is guaranteed sound. I doubt this could be achieved starting from TypeScript types.

I suppose an exploit would be importing an ostensible constant for 'PriceLockPeriod' but that's actually a constant for prototype or not a constant at all. In this sense, the "computed" part of the error message mislead me. Is there a more exact term for what the problem is?

The Jessie rule is correct. The Jessie + rule is there to allow computed numeric lookup, but for brevity without loss of meaningful safety, we accept that + also allows any stringified number including the strings NaN, Infinity, and -Infinity as well as fractions and negative numbers.

That the error message says use leading '+' for strings is bad advice. It should say something like, "if the value is intended to be a numeric index, use + for safety"

Our agoric-sdk Keyword rules are motivated by overlapping safety concerns. Ideally, we could extend Jessie with a rule to also allow computed property access that was guaranteed to be either a number-name or a Keyword. But I don't know of an adequately terse way to do that.

I see now why the rule is necessary. I think this ticket should stay open until the error message is improved.

For runtime changes, your helper with runtime makes sense to me. Also interested in @dtribble 's perspective before writing any code for this.

[michaelfig]

Are you saying PRICE_LOCK_PERIOD is a computed property?

I called it that, because in the ESTree representation that ESLint uses, a[b] vs. a.c is parsed as MemberExpression[computed=true] vs. MemberExpression[computed=false], and the ESLint selectors reflect that difference.

In terms of semantics, it means that a "computed" index (b) is evaluated at runtime, but a "non-computed" index (c) is not.

But I agree, the lint warning should be clarified.