Skip to content
This repository has been archived by the owner on Mar 23, 2024. It is now read-only.

requireDotNotation should ignore reserved words for es5 #161

Closed
ikokostya opened this issue Jan 13, 2014 · 17 comments
Closed

requireDotNotation should ignore reserved words for es5 #161

ikokostya opened this issue Jan 13, 2014 · 17 comments

Comments

@ikokostya
Copy link
Contributor

requireDotNotation should ignore reserved words for ECMAScript 5:

http://www.ecma-international.org/ecma-262/5.1/#sec-11.2

MemberExpression :
  PrimaryExpression
  FunctionExpression
  MemberExpression [ Expression ]
  MemberExpression . IdentifierName

http://www.ecma-international.org/ecma-262/5.1/#sec-7.6

IdentifierName ::
  IdentifierStart
  IdentifierName IdentifierPart

But not for ECMAScript 3 (http://www.ecma-international.org/publications/standards/Ecma-262-arch.htm):

MemberExpression :
  PrimaryExpression
  FunctionExpression
  MemberExpression [ Expression ]
  MemberExpression . Identifier
  new MemberExpression Arguments
Identifier ::
  IdentifierName but not ReservedWord
@markelog
Copy link
Member

markelog commented Feb 1, 2014

What you purposes we do here? Are you satisfied with changes from #167?

@ikokostya
Copy link
Contributor Author

What you purposes we do here?

  1. requireDotNotation by default should work in es5 enviroment and should not report about reserved words in keys of objects. We also need detect strict mode, because in strict mode JavaScript has more reserved words http://www.ecma-international.org/ecma-262/5.1/#sec-7.6.1.2.
  2. We can add new value for this option that enables es3 enviroment and reports about reserved words in keys of objects. But I think more proper decision is to add new option that specifies ECMAScript enviroment and changes work of another options (like es3 option in jshint).

Are you satisfied with changes from #167?

It is preparation work.

@mikesherov
Copy link
Contributor

It is preparation work.

@ikokostya now that #167 is done, and you mention it's preparation work, what is left to do to satisfy this bug? Right now, the only bug seems to be that in non-strict mode, a few false negatives get through. Is that correct?

@ikokostya
Copy link
Contributor Author

  • If I set requireDotNotation: true in es3 enviroment (ie8, ie9)
    obj['while'] = 1 should not report error.
  • If I set requireDotNotation: true in es5 enviroment
    obj['while'] = 1 should report error, because I can write obj.while = 1.

Now requireDotNotation option has only first behavior.

@ljharb
Copy link
Contributor

ljharb commented Sep 27, 2014

Similarly, I have "disallowQuotedKeysInObjects" set to "allButReserved", but when adding "requireDotNotation" set to "true", jscs complains about 'undefined' and 'arguments', both of which are reserved words in ES3.

@qfox
Copy link
Member

qfox commented Nov 5, 2014

@ikokostya what you mean under environment? You mean executing checker in browser?

@ikokostya
Copy link
Contributor Author

@zxqfox Any javascript engine.

@mikesherov
Copy link
Contributor

@ikokostya @ljharb to summarize:

  1. we need an es3 mode, which checks for es3 reserved and future reserved words, and includes arguments and undefined, which aren't technically reserved words but sure act like them for the purpose of property names.
  2. we need to make the default true assume es5 and then don't check the reserved list at all considering es5 allows anything to be a property name.

Does that sound good?

@mikesherov mikesherov added this to the 1.10 milestone Nov 28, 2014
@qfox
Copy link
Member

qfox commented Nov 28, 2014

If you want to have this sets please share them between rules and plugins.

@ljharb
Copy link
Contributor

ljharb commented Nov 29, 2014

Sure, anything that lets me set the base of a project as es3, es5, or es6 would probably work well as a modifier to a number of rules' defaults.

@mikesherov
Copy link
Contributor

Well, we now have esnext as an option. I suppose we could add es3 as an option, and default is assumed es5. @mdevils, thoughts?

@mdevils
Copy link
Member

mdevils commented Nov 29, 2014

Well, currently we have no way to share settings with rules except for their own option values.

@mikesherov
Copy link
Contributor

@mdevils, rules have access to JsFile which has getConfiguration. I suppose certain rules can access a function isES3Enabled, which doesn't currently exist, but would be useful for several rules.

@mdevils
Copy link
Member

mdevils commented Nov 29, 2014

@mikesherov, getConfiguration() is currently at StringChecker. We can pass some configuration to JsFile I think.

@mdevils
Copy link
Member

mdevils commented Nov 29, 2014

@mikesherov, at least JS dialect information is suitable for JsFile, I believe.

@mikesherov
Copy link
Contributor

@mdevils seems like we'd want to know es3/es5/es6 and possible also tabSize as configuration options that jsfile should know about.

@qfox
Copy link
Member

qfox commented Nov 29, 2014

@mdevils @mikesherov 👍 I like this way

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants