prettier
prettier copied to clipboard
prettier
Removes parentheses in expressions with mixed operators
Before:
const foo = (a && b) || c || d;
After:
const foo = a && b || c || d;
The same goes for arithmetic and bitwise operators.
Relevant ESLint rule: http://eslint.org/docs/rules/no-mixed-operators.
Can't seem to reproduce this: https://jlongster.github.io/prettier/#%7B%22content%22%3A%22const%20foo%20%3D%20a%20%26%26%20(b%20%7C%7C%20c%20%7C%7C%20d)%3B%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%7D%7D seems to generate the expected code. Maybe you can try updating your version of Prettier.
@demoneaux Oh, you are right. I updated the description with a correct example. It seems that prettier only removes parentheses when it doesn't lead to a change in logic. Nonetheless, I think it's important to keep the parentheses for clarity if that's possible.
I agree. It should be opinionated about enforcing clear logic between operators. For example, it's just not clear, unless you remember your math classes from HS, that division happens before addition:
const result = 100 / 10 + 10; // 20
const result = 100 / (10 + 10); // 5
const result = (100 / 10) + 10; // 20
It currently enforces unclear operations, when the strong opinion should be to enforce clarity.
A real-world example from some code I'm working on: https://jlongster.github.io/prettier/#%7B%22content%22%3A%22style.left%20%3D%20left%20-%20(this.state.width%20%2F%202)%20%2B%20(parent.offsetWidth%20%2F%202)%3B%5Cn%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3Afalse%2C%22bracketSpacing%22%3Atrue%2C%22doc%22%3Afalse%7D%7D
Both are correct, but I would say that the original is vastly more readable (and one might even say prettier).
Related: https://github.com/lydell/eslint-config-prettier#no-mixed-operators
The create-react-app ESLint config includes the no-mixed-operators warning. So even when the user writes mixed operator code with proper parentheses, Prettier currently converts it to code that causes warnings in create-react-app projects. Since Prettier and create-react-app seem to share some principles and generally work well in combination, it would be a nice design principle if Prettier's formatting rules match create-react-app's ESLint rules.
I think this is one of those cases where prettier should defer to the user's choice. Preserve brackets when they already exist, but don't add new ones.
@erbridge All of this information is lost from the AST. The AST doesn't have any information about extraneous parens, so it's impossible for us to keep the original structure. And in fact, that's the opposite of what we want to do; this project enforces a consistent style by disregarding the original style.
We need to choose whether or not we want to do this, and have it consistent everywhere. I'm a little worried that it would add a lot of noise when it's only helpful in a few situations, but we can talk about it more.
@jlongster Understood. My preference is to get rid of all extraneous brackets. As you say, in general, they would be unnecessarily verbose. If extra clarity is needed, users can pull the sub-statements out into their own variables.
I think that there are many cases when judiciously placed brackets add meaning. For example:
const remainingImageCount = totalImageCount - (currentIndex + 1);
vs
const remainingImageCount = totalImageCount - currentIndex + 1;
I would argue that the first example reads better because is clearly shows the addition applied to currentIndex. By stripping away the brackets, meaning is lost. Sure you could solve this by adding another variable, but I don't think this would make the code more readable.
@Undistraction prettier will keep the parens in that case. Those expressions are semantically different. 5 - (2 + 1) === 2 but 5 - 2 + 1 === 4. Prettier maintains the correct semantics and add parens where needed, naturally.
We are now adding parenthesis around && inside of || which seems like the biggest issue people are facing in this issue. I'm closing this issue. If there are other places where you'd like parenthesis to be kept, please feel free to open another issue.
Can we reopen the issue?
I believe it's still a good thing to keep the parenthesis when 2 operators have the same precedence.
These kind of things might not be an issue to read for you, but for a lot of people, it's way more clear with parenthesis. https://prettier.github.io/prettier/#%7B%22content%22%3A%22(10%20-%205)%20%2B%2025%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3A%22none%22%2C%22bracketSpacing%22%3Atrue%2C%22jsxBracketSameLine%22%3Afalse%2C%22parser%22%3A%22babylon%22%2C%22doc%22%3Afalse%7D%7D
With division and multiplication it's even easier to confuse precedence:
https://prettier.github.io/prettier/#%7B%22content%22%3A%22function%20shareInPercent(total%2C%20part)%7B%5Cn%20%20return%20(part%20%2F%20total)%20*%20100%3B%5Cn%7D%22%2C%22options%22%3A%7B%22printWidth%22%3A80%2C%22tabWidth%22%3A2%2C%22singleQuote%22%3Afalse%2C%22trailingComma%22%3A%22none%22%2C%22bracketSpacing%22%3Atrue%2C%22jsxBracketSameLine%22%3Afalse%2C%22parser%22%3A%22babylon%22%2C%22semi%22%3Afalse%2C%22useTabs%22%3Afalse%2C%22doc%22%3Afalse%7D%7D
Division is a very powerful semantic separator of divisor from dividend in an expression. I have to make an effort to not perceive the multiplication as a part of the divisor.
I suggest putting in all the parens if an expression contains division.
I think this should seriously be considered again. Currently prettier transforms this:
const css = `
max-width: ${(ITEM_WIDTH * 4) + (ITEM_GUTTER * 3)}px;
`;
into this:
const css = `
max-width: ${ITEM_WIDTH * 4 + ITEM_GUTTER * 3}px;
`;
which really cannot be the desired outcome? It doesn't provide any clarity on how to read this code - which should be the goal.
I'd love to see parenthesis in calculation expressions were more than one operator is mixed.
Rule idea: When mixing binary operators, always add parentheses, except for the few ones taught in Maths in school: +, -, *, / and **.
By removing parenthesis from expressions prettier introduces ambiguity, which I hope is not the desired outcome. There is simply no way for prettier to understand the context of e.g. mathematical expressions, only the developer is in a place to make sound judgements about parenthesis to group the expression in logical sections.
I strongly disagree that this is what prettier wants to do, it is not just rewriting code without formatting, it's removing actual intent. Simplifying an expression (double parens) is one thing, but removing them is hurtful (for readability). After all, shouldn't prettier prioritize readability and understandability over all other things?
I think gofmt (formatting for Golang) does this kind of behavior well, here are some examples:
a := 10 * 2 + 5 + 20 / 4 * 2 + 9
b := (10 * 2) + 5 + (20 / 4) * 2 + 9
c := (a - (b + (1)))
// becomes
a := 10*2 + 5 + 20/4*2 + 9
b := (10 * 2) + 5 + (20/4)*2 + 9
c := (a - (b + (1)))
Now, we could argue that b has a superfluous parens around 20/4, but we cannot know what the intent is. c on the other hand does have superfluous parens, but what's the benefit in removing them?
Takeaway from golang example is IMO that we can improve visibility (e.g. by grouping operators of different precednece) but we should not try to simplify (syntactically) expressions needlessly. I also think it can make sense to reduce ambiguity by introducing parens (e.g. given the && / || situation) where there might be (visual) ambiguity.
Here presents my use case in Node.js that encounters this same issue:
Real world use case:
/* A basic line of code to measure time taken of a task performed in Node.js */
const requestStarts = process.hrtime();
/* Do some tasks */
doSomeTasks();
const requestEnds = process.hrtime(requestStarts);
/* no-mixed-operators rule will work here and enforces brackets in the line below. */
const requestDiff = (requestEnds[0] * 1e3) + (requestEnds[1] * 1e-6); /* Convert to ms */
console.log(`Time taken - ${requestsDiff.toFixed(3)}ms`);
Expected outcome after Prettier
const requestDiff = (requestEnds[0] * 1e3) + (requestEnds[1] * 1e-6);
Actual outcome after Prettier
/* no-mixed-operators starts throwing error */
const requestDiff = requestEnds[0] * 1e3 + requestEnds[1] * 1e-6;
In general, I love that prettier removes debate about correct formatting from my PRs.
But removing parens? I understand that there may be some challenges with keeping them but please remember the following -
“Programs must be written for people to read, and only incidentally for machines to execute.”
Harold Abelson, Structure and Interpretation of Computer Programs
So you can sit and think and decide that they're unnecessary, but you're not the person i'm trying to communicate with.
Update here, we do want to add parenthesis back when they help disambiguate code for humans. I haven't had the time to actually go through all the cases and figure out when to actually do it. If someone is interested in tackling this, this would be a welcome addition to prettier.
I reviewed all the comments raised here and related issues and here's what I think the next steps are:
- A few months ago, we added parenthesis when mixing && and ||.
- For +*/-, I don't think that we should put parenthesis. The priority for those are taught in every school in the world. If you find them confusing, I think that taking the time to learn them is going to be useful for your career in the long run.
- For === inside of === (or !===), we should add parenthesis as it's not clear.
a === b === c - For * inside of %, we should add parenthesis.
That's the extent of what I have found people report here. If there are more things, please let me know.
If someone is interested in working on it, that would be great!
For +*/-, I don't think that we should put parenthesis. The priority for those are taught in every school in the world. If you find them confusing, I think that taking the time to learn them is going to be useful for your career in the long run.
I agree with the put part, but the current behavior is to remove, which I do not agree with. I don't think any programmer is going to have a problem with differentiating the priority of +*/-. The real problem lies in code readability. The human mind is prone to simplifying and taking shortcuts, when reading expressions containing multiple operators, it's not a question of knowing the priority, it's a question of how easily you can spot the difference, which segments belong where. In these cases a parenthesis or two or ten can really help disambiguate the statement.
To me it seems like the reason to stick to this choice is that the parens are lost in the AST, but this seems like lousy reasoning to me. It would be OK to mention the behaviour in the README and note that it will be fixed in the future, but making a decision based on current limitations just seems wrong.
Finally, does removing parens make the code any prettier? My opinion is no, it does not.
@mafredri Just keep in mind that Prettier actually doesn't remove your parentheses per se. It throws away all of your code, and then prints equivalent code pretending it never even saw the original code basically (except for a few special cases). That's what makes sure that Prettier can print code consistently regardless of who wrote it. So the crux of this issue is defining the rules for when Prettier should print parentheses. Thinking about how the input code looked is the wrong approach.
When this patch is gonna be released? I hope that this will result in prettier-eslint to be updated because right now it causes conflicts with eslint config.
@vjeux
If you find them confusing, I think that taking the time to learn them is going to be useful for your career in the long run.
IMO, this is a condescending position.
@lydell
Just keep in mind that Prettier actually doesn't remove your parentheses per se. It throws away all of your code, and then prints equivalent code pretending it never even saw the original code basically (except for a few special cases).
I don't wish argue technicalities here, if parens disappear, the are removed, implementation may be what it is. My hope is that issues can be considered in a broader sense than current limitations of the implementation. Just because a parenthesis doesn't modify the behaviour of the code doesn't mean that it's not valid AST . If it's missing from prettier it's because someone decided it shouldn't be there (for better or worse).
EDIT: I realize the above statement can be somewhat incorrect, AST doesn't need to contain parens, but I hope you can understand the underlying intent. More importantly, AST is for computers, code is for humans 🙂.
So the crux of this issue is defining the rules for when Prettier should print parentheses. Thinking about how the input code looked is the wrong approach.
That is not the point I'm trying to make. To me this seems like the wrong approach since prettier is supposed to be a tool to help humans, not fight them. And IMO unnecessarily1 removing parenthesis is hurtful, rather than beneficial, at least it wont make the code prettier or more readable.
1 Please consider this statement, not from the point of prettier AST, but from the eyes of the developer prettier is supposed to help
This should adhere to the rule applied with no-mixed-operators.
When connected to Eslint, this presents a lint issue when prettier is active that is essentially unresolvable, at least without using // prettier-ignore to manually skip it.
When using flow with its stuff as comments (so that the source remains valid javascript), removing unnecessary parenthesis nukes its type assertion expressions.
i.e.
(x /*: number */)
turns into
x /*: number */
which isn't the same thing
@frou That has nothing to do with this issue. Besides, that seems to have been fixed. Please double check that you're using the latest Prettier version.
@lydell This issue was linked from another one as the locus for unnecessary parenthesis stuff, so my comment was not a total drive-by :)
Your link shows a type on a function parameter. The Type Assertion is a different construct and can be on a line/statement all by itself.
For +*/-, I don't think that we should put parenthesis. The priority for those are taught in every school in the world. If you find them confusing, I think that taking the time to learn them is going to be useful for your career in the long run.
There's a big difference between "finding them confusing" and wishing to clearly express one's intent.
not fixed for me in prettier 1.6.1
formatOnSave triggers prettier and is reverting this change.
@codeofsumit That works as intended in Prettier 1.6.1. It was decided to remove parentheses for the common grade school math operators.
@lydell thanks for the update. I'm new to prettier but having the option eslintIntegration: true makes me expect prettier would respect any rules defined in the project specific .eslintrc? Is that not the case?
@codeofsumit Prettier does not look at your ESLint config at all. The recommendation is to use eslint-config-prettier, or, if you disagree with Prettier's formatting, prettier-eslint.
eslintIntegration: true is a prettier-vscode setting to use prettier-eslint instead of prettier.
@azz well I did set eslintIntegration: true so the case in the above screenshot should work, shouldn't it? 🤔
Taking a closer look, no-mixed-operators does not support autofix, so prettier-eslint (and by extension eslintIntegration: true) can't help you. You have to edit your ESLint config: https://github.com/prettier/eslint-config-prettier#no-mixed-operators
@lydell uhm wait a minute. I want the eslint rule to be enforced. I don't expect that prettier is adding the brackets, my issue is that prettier removes the brackets when I add them. Can I change this?
@codeofsumit There's no way to change this. You have to follow the advice I linked to (refactor your code): https://github.com/prettier/eslint-config-prettier#no-mixed-operators
@lydell ah sorry, they're specifically talking about prettier 👍 . Ok thanks. Will find something...
@leonid-shevtsov posted this example in this thread before but the importance of it has been buried by comments about aesthetics or readability: https://prettier.io/playground/#N4Igxg9gdgLgprEAuEAzArlMMCW0AEAzgBYCGATnAJJQAKc5YCMAFDBDKQDYA0+ADhRgBKYAB0o+fJRjpykloPIx8AenztOXYfgBU+AIwAGIwG4JAXxA8QEfrmiFkofuRywA6jgAmMYsgAOIxtOACMvX38kACYbQncAcy44AEV0DjhkVG5COBDyUhwuRIBhCABbctJkEChoTJtQgrAAazgYAGVBMETkGHJ0PJAAK0IADwAhZrbO0nK4ABl3TKRsrlybJVzyGtDSUIBPLmhrEFzynCycofRcgBV9p1Xrm28IMCv1odJCGE+N2zoGD8IHRf5wCwWIA
I'll post the example in the comment instead of just the playground. This is how it's originally written (and I how I write code for calculating percentages):
function shareInPercent(total, part){
return (part / total) * 100;
}
This is what prettier converts it to:
function shareInPercent(total, part) {
return part / total * 100
}
This is an error. If you have prettier enabled on save, you're lucky to catch it. If you have it on a pre-commit hook, you'll miss it. Granted, you can teach devs to make sure to write it like this (part * 100) / 100 but it's hard to reverse years of using the previous pattern. Across a team, it's bound to cause errors that aren't caught.
I know this may be a difficult problem to solve, but I want to raise it's status of importance because it's causing real errors in people's code.
@jefffriesen how is it erroneous? There is no input for part and total that would produce a different result in the example and "prettified" version. Semantically it is correct.
@mafredri You're right.
I definitely have seen errors but I keep blaming it on this. That's kind of embarrassing. Multiplication and division have the same precedence, but in C-style languages, the operation proceeds from left to right. It's still uncomfortable though, because in handwritten math, it looks like what's happening is this:
// If I wrote this by hand:
1 / 2 * 100 = 50
// It would, in my experience, be interpreted as:
1 / (2 * 100) = 0.005
Ok, so this issue is actually a readability problem, not a correctness problem.
I didn't mean that prettier breaks code. The code works fine.
It's us humans who have a hard time understanding the expression without brackets because for humans the division operator is a strong separator of what's being divided from what's it divided by.
Since prettier's goal is to make code nicer for humans, it should definitely acknowledge that for humans, division has lower precedence than multiplication.
(Which - actually - can be a great simple solution to this problem: put division into a precedence class of its own, between multiplication and addition.)
Here is another simple example to add to the discussion:
Before
const r = (a * b + c) + (x * y + z);
After:
const r = a * b + c + (x * y + z);
I don't think there is any argument for the transformed code being easier to parse than the original. In this case prettier has made the code harder to read - it is less clear that there are two very similar parallel calculations being performed and summed.
Just weighing in here:
I think it's a crutch for prettier to format code like this:
if (x || (y && z)) { }
Programmers should know that && has a higher precedence over ||. Adding the parentheses is a crutch and could likely hinder their understanding of JavaScript. I know this is a bit pedantic, but I wanted to throw my two cents in anyway.
@ffxsam If you read through the thread you'll see that Prettier used to work like you suggest, before taking in the overwhelming community feedback to actually do output parens in that case. This decision has already been made.
My opinion on confusing * /:
This
return (part / total) * 100;
Is definitely more readable than this:
return part / total * 100;
And here's why:
It's not only a question of understanding mathematical operators. In my opinion, the second one is more confusing because even when you know how it works, it might look like the one who wrote this code meant something else.
I think a good analogy is switch statements and how everyone dislikes them for the fact that they implicitly fall through if there's no break or return. The problem is that when someone intentionally writes a switch statement that is supposed to fall though, the reader of the code might assume that it's forgotten by mistake.
Same here.
Adding explicit parenthesis shows intention and therefore removes possible confusion.
I totally get these points, and I think they’re valid. But it seems like more ESLint’s territory, not Prettier’s. As I understand it, Prettier is concerned with formatting, ESLint is concerned with style. Other than changing spacing and line wraps, and the very few stylistic points (dangling commas, single vs double quotes—which are entirely configurable, BTW), Prettier lets the developer decide how to style their code.
In fact, there are some cases where Prettier removes parentheses against the developer’s will, because it decides they’re not necessary. For instance:
const myFunc = (arg) => {
// code
};
Prettier will change this to:
const myFunc = arg => {
// code
};
But maybe I was intentionally wrapping my single argument with parentheses so it makes my transition to TypeScript later a little easier (const myFunc = (arg: string) => {}).
In the end, I don't mind so much if parentheses are added to make mixed operators more clear. I'm all for code legibility. My concern is, will this set a precedent where Prettier forces more stylistic choices on the developer? IMO these should be configurable as with trailing-comma, single-quote, etc.
Perhaps this might be a good place for a disabling comment. But not ignore the line fully.
Instead, it might be a good place for a comment like:
// prettier preserve-parens
...or something like this.
@ffxsam Why do you think Prettier should not be concerned about style? Ideally it would only use the AST to print the code (not looking at the original at all) so Prettier should make all the stylistic choices for you (e.g. add parens around single arg arrows or not, or parens around binary expressions). But since the JS community has already embraced a few "blessed" coding styles (like standard), we enabled some options to unblock adoption by a large number of people.. but that doesn't mean everything should be configurable.
Using Prettier is more about giving up control of your formatting (whenever possible).. and if you do want to control, there are tools to format your code after it was prettified (prettier-eslint, prettier-standard, and others) :)
@everdimension I don't think that // prettier-ignore would suffice for this case, if you want that kind of control over formatting.
This seems to be a related case. Prettier changes this (dummy example):
const newObj = {
...(includeOptions ? options : {})
};
to this:
const newObj = {
...includeOptions ? options : {}
};
Admittedly, there are other, arguably better, ways to write the original code. Still, the result in this case seems less readable than the original.
@vjeux I just tried to reproduce this in the playground and can't seem to do so. An analogous (but somewhat more complicated) situation just happened in a codebase I'm working on, though; I'll see if I can figure out what exactly may have triggered it (and yes, I'll open an issue if I can do so).
@vjeux Ah, it seems to happen only when using the TypeScript parser specifically. Is it still appropriate to open a separate issue here in that case?
I'm getting into prettier and the stripping of parens is one reason I may stop using it completely. Parens help me to visually parse my code more easily. I'd like some way to keep them in the code when they are added (no need to automatically add them).
When using Await statements to call an API that returns json I often write some code like this: var result = (await client.request(q.user.checkUsername(), {username}).catch(console.log)).User
In the example above, I want to return the .User from the object. Sometimes I might need the object itself, so I can write: var result = (await client.request(q.user.checkUsername(), {username}).catch(console.log)) Which Prettier turns into var result = await client.request(q.user.checkUsername(), {username}).catch(console.log
I'd rather Prettier just leave the Parens, to keep it visually consistent with my other code, and make it easy for me to parse elements from the returned object. Visually, it's easier for me to understand the await statement as a logical operation that happens before other things.
Hi @jdheeter, could you open a new issue around your specific case using the prettier playground to demonstrate? There are instructions for using the playground in the issue template.
Looks like that’s already kept:
Prettier 1.10.2 Playground link
Input:
async () => {
var result = (await foo().catch(bar)).baz
}
Output:
async () => {
var result = (await foo().catch(bar)).baz;
};
Here is an example of a situation where I want to keep the parens:
Prettier 1.10.2 Playground link
Input:
async () => {
var result = (await foo().catch(bar))
}
Output:
async () => {
var result = await foo().catch(bar);
};
@jdheeter Would you write
var result = (foo + bar)
as well? (Reminds me of lisp.) Why would await expressions be different?
I would not.
The result of + is going to be a number or a string and happens sync. The result of an await expression can be anything and needs to happen before any other operations. In my instance, the result is an object which I often need to pick some data out of, (but not always).
I agree that it's a specific stylistic choice, I just assumed Prettier would give me an escape hatch to format my code in a way that is more readable.
I just assumed Prettier would give me an escape hatch to format my code in a way that is [whatever]
There are // prettier-ignore comments. Other than that, there’s no way to format the code whatever way you wish – that’s the whole point of Prettier ;)
I agree, it's not configurable in prettier, (like it is in eslint no-mixed-operators rule)
not really happy that
if (!inputColon && inputKey || inputColon && inputValue) return;
is converted in
if ((!inputColon && inputKey) || (inputColon && inputValue)) return;
I'm throwing my hat in the ring in favor of preventing the automatic removal of parentheses with mixed operators. As multiple people before me have already stated, leaving the parentheses helps make the code more readable.
Prettier is an awesome tool, and this is probably the only (minor) thing I don't like about it.
Any update on this guys? Just wish it would keep parenthesis for mixed operators like (100 / 5) * 3. Playground
This reminds me of the older issue concerning the automatic removal of parentheses around JSX returned from arrow functions - which was another symptom of Prettier unfortunately favoring code concision over code readability. I'm looking forward to the day I can use Prettier, but until then, it's no-brainer issues like these that give me cold feet. Such a tool simply shouldn't perform any formatting that introduces cognitive load for anyone reading the code.
I'm excited to see things getting better with each release though - all the hard work is greatly appreciated! :)
Our team would also appreciate if this were configurable 👍 Parenthesis just help reduce the cognitive load when scanning code, (a / b) * c is a lot easier (in my opinion) to mentally consume than a / b * c
You know, after thinking about this some more, I've come around from my previous stance. In fact, I now think that prettier doesn't go far enough. After all, since multiplication and addition are commutative, in order to ensure a consistent style, prettier should sort the operands:
Why allow both
const volume = width * length * height
and
const volume = length * width * height
when the canonical expression can clearly be arrived at by sorting (by name for variables and magnitude for numbers)?
const volume = height * length * width
I stopped using prettier: consistency != readability I feel validated by the official Google style guide, which suggests using parens to improve readability. https://google.github.io/styleguide/jsguide.html#formatting-grouping-parentheses
@uhoh-itsmaciek due to how floating points are implemented, moving around operands will not product the exact same result. In most JavaScript applications it doesn’t really matter but if you are doing modeling computations (machine learning, physical analysis ...) those kind of things start to matter.
I absolutely understand the philosophy behind prettier, but this instance I think its a clear case of pedantry over pragmatism. When it comes to bracket removal prettier is making code harder to parse in exactly the places where code needs to be easy to parse.
I don't understand why this is still an issue and not being fixed across one year.
Reopening since many people don’t like the way things are formatted right now.
@thalesfsp Prettier is an open-source project. We need people like you to help come up with a heuristic that prints parentheses in cases where they improve readability, doesn’t print them when they don’t improve readability, and isn’t affected by coders’ under- or overuse of parentheses? Remember, many — if not most — of us are volunteers doing this in our spare time.
We need people like you to help come up with a heuristic that prints parentheses in cases where they improve readability, doesn’t print them when they don’t improve readability, and isn’t affected by coders’ under- or overuse of parentheses?
At risk of repeating myself, I feel this is the wrong conclusion and I don't really understand why this issue was re-opened if this is the case. Most issues raised here are due to the fact that prettier can't know the developers intent.
IMO the only way forward is to make parenthesis part of the AST, some rules for simplification are allowed (e.g. double parens), but should not remove groupings as intended by dev.
My suggestion: There should be a option for disabling Prettier from removing the surplus parentheses, e.g. removeSurplusParentheses: true (default), so if I set it to false, Prettier will only format my code and won't remove the surplus parentheses that are used for readibility.
@cassianomon from https://github.com/prettier/prettier/issues/187#issuecomment-313303188
Just keep in mind that Prettier actually doesn't remove your parentheses per se. It throws away all of your code, and then prints equivalent code pretending it never even saw the original code basically (except for a few special cases)
@duailibe That's what is being discussed in this issue, for Prettier to keep the readibility of the code.
from https://github.com/prettier/prettier/issues/187#issuecomment-310554650
“Programs must be written for people to read, and only incidentally for machines to execute.” Harold Abelson, Structure and Interpretation of Computer Programs
@cassianomon I know what's being discussed. I pasted that quote to show you that your suggestion doesn't make sense for Prettier, since Prettier does not remove anything from your code, we rewrite entirely. We can't prevent anything from being removed, but we can make it print in a way that makes sense. It's a subtle difference and that's what I'm trying to emphasize.
If you do want those parens printed, please suggest how we can detect when a paren is necessary for readability and when it's not. That's the way we can improve Prettier's output.
@cassianomon What @duailibe means is that the process doesn't involve taking your code and selectively removing parts. It involves converting it all into an AST, then printing out your code again based on the AST. He isn't saying it literally doesn't remove anything, but that the process doesn't involve selective removal, but a transformation to an AST, then back to JS. It's like its being reduced to the minimum possible code (while still retaining the original meaning) and then rendered, so the extra brackets (which don't change the meaning) are thrown away and no longer exist. So to handle this issue there needs to be a heuristic in place to decide how the bare-bones, bracketless version should be rendered with brackets.
@Undistraction I understood what Prettier does, I insist in my suggestion, since I can't use prettier at my work because it is removing the parentheses that are used for readibility, I think this falls on this issue.
Perhaps if people posted one example each of the worst case they've encountered in real world code while using Prettier we could see a pattern and find a better middle ground. Right now it is very had to tell what people actually need. Let's take one small step at a time.
@lydell One real world example is if the airbnb style guide (arguably pretty popular itself) is being used by a team as the base eslint configuration for a project. Grade school math or not I still see the benefit in enforcing the avoidance of mixed operators, but that's just personal preference. Either way I think it's fair to say a strong eslint configuration to ensure consistent coding style within a team is a good and more customizable option, not to mention it can be determined on a per project basis.
Exhibit A: Airbnb JavaScript Style Guide / eslint-config-airbnb Exhibit B: no-mixed-operators
@ImAdamTM We are aware of the conflicts with other style guides. @lydell asked for
one example each of the worst case [you’ve] encountered in real world code
(emphasis mine)
Can you provide an actual code example where Prettier significantly harms readability and how you would format it?
@j-f1 thanks for your continued interest in addressing this! I think the original report contains a good example:
const foo = (a && b) || c || d;
is turned into
const foo = a && b || c || d;
and the original is more readable. Okay, technically this is not real world code, but it's not exactly a contrived example either. This comment includes another good example:
const css = `
max-width: ${(ITEM_WIDTH * 4) + (ITEM_GUTTER * 3)}px;
`;
becomes
const css = `
max-width: ${ITEM_WIDTH * 4 + ITEM_GUTTER * 3}px;
`;
Again, the original is more readable.
A lot of the "just think about operator precedence when reading code" responses seem to ignore the value that clear formatting brings to the table in the first place.
@ImAdamTM please note that the next (semver-major) version of the airbnb config will have an altered configuration for no-mixed-operators, so that specific case may no longer apply. Additionally, to use the airbnb config with prettier, you have to/will have to use prettier-eslint, so that eslint can clean up some of prettier’s output.
The cases that we find, at airbnb, where the effective removal is problematic, are when the choice to place otherwise-unnecessary parens is intentional by the developer, to disambiguate and make their intentions clear. In these cases, it is impossible to reconstruct the need for these parens from the AST - as such, the only way to “preserve” them is to literally do that: to keep track of them, in a side channel from the ast, and when printing, add them in as needed.
@uhoh-itsmaciek The &&/|| case has already been changed/fixed.
Prettier 1.12.1 Playground link
Input:
const foo = (a && b) || c || d;
Output:
const foo = (a && b) || c || d;
to use the airbnb config with prettier, you have to/will have to use prettier-eslint, so that eslint can clean up some of prettier’s output.
Or eslint-config-prettier :) (Depending on which tool you'd like to "win".)
Here is a simple example (from earlier in the thread) which I think is a good one:
Prettier 1.12.1 Playground link
Input:
const r = (a * b + c) + (x * y + z);
Output:
const r = a * b + c + (x * y + z);
Hard to say how real-world that example is though because of all the one-letter variables. At this point I really do think we need real-world examples to be able to make a good decision.
Sorry to jump in, but here's two examples in one of my apps where the extra parenthesis make the code more readable to me personally:
1
-
With parenthesis. Notice how the unary plus is being applied to the final result
-
Without parenthesis.
2
-
With parenthesis surrounding the division operation.
-
Without parenthesis. The red squiggly underlines are due to
no-mixed-operatorsrule.
