New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Idempotency issues #339

Closed
olafurpg opened this Issue Jun 22, 2016 · 3 comments

Comments

Projects
None yet
1 participant
@olafurpg
Member

olafurpg commented Jun 22, 2016

My previous claims that scalafmt is idempotent were false. I just noticed a bug in the property tests. It appears that 81 files out of the 9.350 files in the mega corpus tests still trigger non-idempotent formatting. Here's the diff by running scalafmt twice on those files: https://gist.github.com/olafurpg/7d95a03f1598acff5ace7552a38b1e54

0.2.6 already fixed a whole bunch of idempotency issues so I definitely recommend you upgrade to 0.2.6. Fixing the remaining idempotency issues will be a top priority for 0.2.7.

@olafurpg olafurpg added the bug label Jun 22, 2016

@olafurpg olafurpg added this to the 0.2.7 milestone Jun 22, 2016

olafurpg added a commit that referenced this issue Jun 22, 2016

@olafurpg olafurpg modified the milestones: 0.2.7, 0.2.8 Jun 23, 2016

olafurpg added a commit that referenced this issue Jun 29, 2016

Stop relying on tok1.start - tok2.end, towards #339
Decreases non-idempotent cases in test suite from 14 to 11.

@olafurpg olafurpg self-assigned this Jun 29, 2016

olafurpg added a commit that referenced this issue Jun 29, 2016

Stop relying on tok1.start - tok2.end, towards #339
Decreases non-idempotent cases in test suite from 14 to 11.

@olafurpg olafurpg modified the milestones: 0.2.10, 0.2.9 Jun 29, 2016

@olafurpg olafurpg modified the milestones: 0.2.12, 0.2.11 Jul 8, 2016

@olafurpg olafurpg removed this from the 0.2.13 milestone Aug 6, 2016

@olafurpg olafurpg removed the in progress label Sep 27, 2016

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Oct 23, 2016

Member

As of 0.4.7, if the sources are already formatted with scalafmt, there are 0 cases of non-idempotent formatting in scala-repos. If the sources are not formatted with scalafmt, there are 8 cases of non-idempotent formatting.

I'm embarrassed to make this recommendation, but if you are formatting an existing codebase with scalafmt for the first time, run scalafmt twice.

Member

olafurpg commented Oct 23, 2016

As of 0.4.7, if the sources are already formatted with scalafmt, there are 0 cases of non-idempotent formatting in scala-repos. If the sources are not formatted with scalafmt, there are 8 cases of non-idempotent formatting.

I'm embarrassed to make this recommendation, but if you are formatting an existing codebase with scalafmt for the first time, run scalafmt twice.

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg Dec 26, 2016

Member

I investigated this a bit more and found that optIn.breakChainOnFirstMethodDot = true (default as of 0.5.0) in combination with a acceptNoSplit = true field in Split can cause non-idempotent formatting. I don't see a clear way to fix that case without removing those features.

Member

olafurpg commented Dec 26, 2016

I investigated this a bit more and found that optIn.breakChainOnFirstMethodDot = true (default as of 0.5.0) in combination with a acceptNoSplit = true field in Split can cause non-idempotent formatting. I don't see a clear way to fix that case without removing those features.

@olafurpg

This comment has been minimized.

Show comment
Hide comment
@olafurpg

olafurpg May 12, 2018

Member

Superseded by #917, I don't think it is worth investing more time in adding fundamental improvements to the current architecture.

Member

olafurpg commented May 12, 2018

Superseded by #917, I don't think it is worth investing more time in adding fundamental improvements to the current architecture.

@olafurpg olafurpg closed this May 12, 2018

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