Skip to content
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

FreezingArchRule generates file missing newline #1057

Closed
pkubowicz opened this issue Feb 8, 2023 · 2 comments · Fixed by #1075
Closed

FreezingArchRule generates file missing newline #1057

pkubowicz opened this issue Feb 8, 2023 · 2 comments · Fixed by #1075

Comments

@pkubowicz
Copy link
Contributor

pkubowicz commented Feb 8, 2023

To reproduce:

  1. use FreezingArchRule
  2. run ArchUnit test to let the rule generate file in archunit_store/
  3. commit the file to Git repository
  4. see the diff using git show
  5. open the file in IntelliJ, make any change (remove a random line), save, commit
  6. see the diff using git show
  • Expected: nice Git diff
  • Actual: FreezingArchRule generates a file that shows in Git diff as \ No newline at end of file (step 4)

Additionally, well-behaving editors like vim or IntelliJ put newline character at the end of text files. This is because a file without final newline is not a proper text file. Anyway, as result, if you manually edit generated file (step 5), you generate a fake change in the last line by inserting the missing newline (step 6). Run ArchUnit test again, and you will play ping-pong with Git, doing nonsense changes to the last line.

Suggested solution: Change com.tngtech.archunit.library.freeze.ViolationStoreFactory.TextFileBasedViolationStore#write and stop join ing violations with "\n". Just append \n after each violation and all will be fine.

I can open a PR if you don't have time to fix, just let me know you're fine with the solution.

@codecholeric
Copy link
Collaborator

I agree, I've also stumbled over this several times and was annoyed 😒 Originally the file wasn't thought to be public API so I didn't put too much thought into the concrete layout, since I thought it would only be read programmatically anyway. I'd be happy to accept a PR, or I can also do it in time for the next release 🙂

@codecholeric
Copy link
Collaborator

And yeah, your solution sounds reasonable 👍 Don't see any pitfalls as long as reading the lines again accounts for it (which it might already do anyway 🤷‍♂️)

pkubowicz added a commit to pkubowicz/ArchUnit that referenced this issue Mar 2, 2023
For a file to be considered a text file, it needs to end with
a newline character. Previously, ArchUnit did not add the final newline,
which broke Git diff and other tools.

Additionally, remove unnecessary list allocation: 1 when reading
and 1 when writing a violation store file.

Resolves TNG#1057
pkubowicz added a commit to pkubowicz/ArchUnit that referenced this issue Mar 2, 2023
For a file to be considered a text file, it needs to end with
a newline character. Previously, ArchUnit did not add the final newline,
which broke Git diff and other tools.

Additionally, remove unnecessary list allocation: 1 when reading
and 1 when writing a violation store file.

Resolves TNG#1057

Signed-off-by: Piotr Kubowicz <piotr.kubowicz@gmail.com>
codecholeric pushed a commit to pkubowicz/ArchUnit that referenced this issue Mar 19, 2023
For a file to be considered a text file, it needs to end with
a newline character. Previously, ArchUnit did not add the final newline,
which broke Git diff and other tools.

Additionally, remove unnecessary list allocation: 1 when reading
and 1 when writing a violation store file.

Resolves TNG#1057

Signed-off-by: Piotr Kubowicz <piotr.kubowicz@gmail.com>
codecholeric pushed a commit to pkubowicz/ArchUnit that referenced this issue Apr 15, 2023
For a file to be considered a text file, it needs to end with
a newline character. Previously, ArchUnit did not add the final newline,
which broke Git diff and other tools.

Additionally, remove unnecessary list allocation: 1 when reading
and 1 when writing a violation store file.

Resolves TNG#1057

Signed-off-by: Piotr Kubowicz <piotr.kubowicz@gmail.com>
codecholeric added a commit that referenced this issue Apr 15, 2023
For a file to be considered a text file, it needs to end with a newline
character. Previously, ArchUnit did not add the final newline, which
broke Git diff and other tools.

Additionally, remove unnecessary list allocation: 1 when reading and 1
when writing a violation store file.

Resolves #1057
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 a pull request may close this issue.

2 participants