Skip to content

Conversation

@Sawtaytoes
Copy link

@Sawtaytoes Sawtaytoes commented Feb 28, 2020

I added the missing --fix functionality.

Using --fix requires indent from ESLint for it to make sure indentation is correct after fixing.

Apparently the other newline fixes in ESLint have the same issue of requiring indent to properly reformat on fix.

This requires `indent` from ESLint for it to make sure indentation is correct after fixing. Apparently the other newline fixes in ESLint have the same issue of requiring `indent` to properly reformat on fix.
@Sawtaytoes Sawtaytoes mentioned this pull request Feb 28, 2020
@getify
Copy link
Owner

getify commented Feb 28, 2020

14 of the PARENS tests are failing, as indicated here:

https://travis-ci.org/getify/eslint-plugin-proper-ternary/jobs/656153042

Can you check to see why?

@Sawtaytoes
Copy link
Author

Sawtaytoes commented Feb 28, 2020

@getify Took me a while, but I figured it out. No fix yet.

This is adding parens only to sub-expressions in ternaries:

"ConditionalExpression": function enter(node) {
  for (let clause of [node.test,node.consequent,node.alternate,]) {
    let exprType =
      (clause.type == "ConditionalExpression") ? "ternary" :
      (clause.type == "BinaryExpression" && ["==","===","!=","!==","<",">","<=",">=","in","instanceof",].includes(clause.operator)) ? "comparison" :
      (clause.type == "LogicalExpression") ? "logical" :
      (clause.type == "UnaryExpression" && clause.operator == "!") ? "logical" :
      (["CallExpression","NewExpression",].includes(clause.type)) ? "call" :
      (["ArrayExpression","ObjectExpression",].includes(clause.type)) ? "object" :
      (["Identifier","MemberExpression","Literal","TemplateLiteral",].includes(clause.type)) ? "simple" :
      "complex";

For whatever reason, my fixes have an assumption these sub-expressions are only ConditionalExpression; therefore, it sometimes tries to grab sourceCode.getTokenBefore(clause.test) which only works if this is a ternary.

To pass all tests, this could be modified like so:

const tokenBefore = sourceCode.getTokenBefore(clause)
const tokenAfter = sourceCode.getTokenAfter(clause)

@Sawtaytoes
Copy link
Author

Sawtaytoes commented Feb 28, 2020

Yep, that's the fix; although, my changes add newlines even to single-line statements. That'll also need to be addressed before a merge.

Lastly, there's an issue when blocks (array, functions, and objects) are wrapped. I'll be adding that functionality as well.

@Sawtaytoes
Copy link
Author

Sawtaytoes commented Feb 28, 2020

I added the ability to keep it single-line with addParensOnSameLine and added another one (func) allowing you to disable parens for nested functions.

This is ready for merge :); although, it's missing tests and documentation :/.

@getify
Copy link
Owner

getify commented Apr 13, 2020

Thanks for getting the existing tests to pass.

Definitely want to get the new tests in, so we can validate that it's doing what's expected in all the various cases. After that, we can document it.

If you're able to add new tests, that's appreciated. I haven't been able to circle back to this and work on it, regrettably.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants