Ship the byte order mark removal script - #9
Open
henrikottesorensen wants to merge 4 commits into
Open
Conversation
2.2 switched SA1412 off, so consumers no longer have to carry a mark on every source file - but nothing removes the ones they already have, and doing it by hand across a tree is where mistakes get made. Stripping the first three bytes blindly destroys a UTF-16 file, whose mark is the only record of its encoding. tools/de-bom.sh reports by default and changes nothing until asked. It refuses to run on a dirty tree, so what it does lands as one commit git checkout can undo. It skips UTF-16, .sln, and the usual build directories, preserves file modes by writing back into the original rather than moving a temp over it, and handles paths with spaces - which matters, because "Service References" has one and an earlier draft reported every file under it as unreadable. POSIX sh with no GNU-isms: od rather than xxd, which ships with vim and is not on every container, and no sed -i, which needs a mandatory argument on BSD and refuses it on GNU. Exercised end to end under a PATH containing only BSD utilities as well as under GNU, against a tree with spaced paths, a UTF-16 file, a mode-640 file and a dirty checkout. The readme says to upgrade before running it. On 2.1.x SA1412 still demands the mark, so stripping first breaks the build on every file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It predates everything here and nothing enforces marks in either direction now, so it was harmless - but a repository shipping a script to remove them should not be carrying one. Removed with that script, which is also the first time it has been run against something other than a test tree: one file, one line, and the build, all six rules and the packaging check unchanged afterwards. The upgrade notes gain the second half of the ordering problem. Take 2.2 before stripping marks, which was already there - and then close the IDE while stripping them, which was not. Visual Studio and Rider fix a file's encoding when they open it and keep it for the buffer, so a file opened with a mark has one written back on the next save regardless of what is now on disk. An editor left running undoes the script quietly, file by file, as you touch them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The other team's code has been under Notalib for years, and one root does not cover the other: a prefix only matches at a dot boundary, so Notalib.Something is not under "Nota" - the same rule that stops "System" swallowing "SystemsManager". Without both, half of Nota's own code sorted among the vendors. This is also the case that cannot be inferred. A file in Nota.Something has no way to know Notalib is ours too, which is exactly why the setting exists alongside the inference. Verified against the built globalconfig: with both roots, Nota.* and Notalib.* both land in the last block. Worth knowing, since separate_roots is on: they are two roots, so they get a run each rather than sharing one. Nota.* and Notalib.* run together is reported as UA1001. That is the setting working as asked rather than a surprise, but it is a visible change in every file that imports both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
0.3.0 makes first_party_prefixes optional: with nothing configured, each file is judged against the namespace it declares. That does not change anything here, because this package sets the prefixes explicitly and configuration still wins - which is the point of taking it. A consumer who has never set them stops having their own code sorted among their vendors, quietly, for want of a line in .editorconfig. It also raises the floor on which SDKs the using rules load at all: 0.3.0 inherits 0.2.1's build against Roslyn 4.8, so anything from the .NET 8 SDK upwards runs them rather than skipping them with CS9057. Nota and Notalib still have to be named. A file in Nota.Something has no way to know Notalib is ours too, which is exactly the case inference cannot reach. Verified rather than assumed: builds clean, all six rules report, the encoding check passes, the packaging check passes, and the packed nuspec declares UsingLayoutAnalyser 0.3.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
2.2 switched
SA1412off, so consumers no longer need a byte order mark on every source file. Nothing removes the ones they already have, though, and doing it by hand across a tree is where it goes wrong — stripping the first three bytes blindly destroys a UTF-16 file, whose mark is the only record of its encoding.tools/de-bom.shreports by default and changes nothing until asked:It refuses to run on a dirty tree, so the result is one commit
git checkoutcan undo. It skips UTF-16,.slnand build directories, preserves file modes by writing back into the original rather than moving a temp over it, and handles paths with spaces — which matters, becauseService Referenceshas one, and an earlier draft reported every file under it as unreadable.POSIX
sh, no GNU-isms:odrather thanxxd, which ships with vim and is not on every container, and nosed -i, which needs a mandatory argument on BSD and refuses one on GNU. Run end to end under a PATH containing only BSD utilities as well as under GNU, against a tree with spaced paths, a UTF-16 file, a mode-640file and a dirty checkout.The readme gains an Upgrading from 2.1 section, including the ordering trap: take 2.2 first, because on 2.1.x
SA1412still demands the mark and stripping early breaks the build on every file.Also here
Nota.CodeAnalysis/Nota.CodeAnalysis.csprojcarried a UTF-8 BOM of its own, predating all of this. Harmless — nothing enforces marks in either direction now, andNOTA0001is content because a mark is valid UTF-8 — but a repository shipping a script to remove them should not be carrying one. Removed with that script, which is its first run against something that is not a test tree.The upgrade notes also gain the second half of the ordering problem: take 2.2 before stripping marks, and close the IDE while stripping them. Visual Studio and Rider fix a file's encoding when they open it and keep it for the buffer, so an editor left running writes the mark back as you touch files — undoing the script quietly rather than failing.
Co-Authored-By: Claude Opus noreply@anthropic.com