Fail codegen for powered unit prefixes

[angularsen]

Motivation

Follow-up to #1700.

We should fail codegen when Prefixes is used on units that look like powered units themselves, such as SquareMeter, CubicMeter, or abbreviations starting with m², m³, ft³, and similar symbols.

That avoids reintroducing bugs like KilocubicMeter formatting as km³, where the generated prefix looks like it applies before the length unit is cubed.

This is intentionally narrower than checking base dimensions. Units like Megawatt, Megajoule, and Megaohm still need to be allowed even though their SI base dimensions contain exponents internally.

Changes

• Add a CodeGen guard for Prefixes on units that look like powered units.
• Keep existing valid derived-unit prefixes working.
• Document that this rule is now enforced by CodeGen.

Validation

• dotnet build CodeGen\CodeGen.csproj
• generate-code.bat
• Temporarily added Prefixes: [ "Kilo" ] to Area.SquareMeter and verified CodeGen fails with the new error, then reverted the temporary edit.

[claude]

Review: Fail codegen for powered unit prefixes

Nice, tightly-scoped follow-up to #1700 for adding a guard so this class of bug (km³, mm³/kg ambiguity) cannot be reintroduced silently.

Breaking changes
None. The diff only touches CodeGen/Helpers/PrefixBuilder/UnitPrefixBuilder.cs and docs -- no Common/UnitDefinitions/*.json changes, so no generated code or public API is affected. This is a build-time guard only; it will only ever fail a future build, never change existing runtime behavior.

New quantities/units
None added.

Generated code changes
None in this PR, so nothing to review there.

Style and conventions
Follows existing patterns well: UnitsNetCodeGenException is used the same way as in QuantityRelationsParser.cs / UnitEnumValueAllocator.cs, and the regex is static readonly plus Compiled/CultureInvariant, so there is no per-call overhead at codegen time.

Possible issues / suggestions

1. Redundant check inside the prefix loop: ThrowIfPrefixesAreUnsafeForPoweredUnit(quantity, unit) (UnitPrefixBuilder.cs:67) only depends on unit, not on prefix, but it is called once per prefix inside foreach (Prefix prefix in unit.Prefixes). For a unit with multiple prefixes it re-runs the same (cheap) check redundantly. Not a correctness issue, just slightly misplaced -- could be hoisted to once per unit, e.g. right after entering the outer foreach (Unit unit in quantity.Units) loop, guarded by if (unit.Prefixes.Count > 0).

2. Exception gets rewrapped generically: the new UnitsNetCodeGenException thrown from ThrowIfPrefixesAreUnsafeForPoweredUnit is caught by the surrounding catch (Exception e) (line 84) and rethrown as a plain Exception with a redundant "Error parsing prefix {prefix} for unit ..." message that duplicates info already in the inner exceptions message. Not wrong, just noisier than the other UnitsNetCodeGenException call sites, which propagate directly. Minor.

3. Heuristic is intentionally narrow (as called out in the PR description): LooksLikePoweredUnit only catches units named Square*/Cubic*, or whose own abbreviation starts with a superscript 2/3/4 or ^2/^3/^4. That is a reasonable, pragmatic scope for this fix, but it does mean a future powered unit that does not follow the Square/Cubic naming convention (or whose abbreviation does not lead with the superscript) could still slip through undetected. Worth a short comment on LooksLikePoweredUnit noting this is a naming-convention heuristic, not a dimensional-analysis check, so future unit definitions are not assumed to be fully covered by it.

4. Verified the regex against the current Common/UnitDefinitions/*.json: checked all 90 files that use Prefixes today, and none have a Square*/Cubic* name or a leading powered-symbol abbreviation, so this guard is safe to land without breaking the existing build.

Test coverage
There is no CodeGen test project in the repo today, so the manual validation described in the PR body (temporarily adding Prefixes: ["Kilo"] to Area.SquareMeter, confirming the new error, then reverting) is consistent with how the rest of CodeGen is validated currently. If a CodeGen.Tests project gets introduced down the line, this would be a good candidate for a direct unit test on UnitPrefixBuilder (feed it a fake Quantity/Unit with Prefixes set on a Square*/m³-abbreviated unit and assert the exception).

Performance / Security
No concerns. This only runs during codegen, not at runtime, and there is no security-sensitive surface here.

Overall: a solid, low-risk guard that matches the intent described in the PR. The two items above (redundant per-prefix check, generic exception rewrap) are minor polish, not blockers.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96%. Comparing base (c1f1438) to head (b8854cd).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@          Coverage Diff           @@
##           master   #1702   +/-   ##
======================================
  Coverage      96%     96%           
======================================
  Files         457     457           
  Lines       29301   29301           
======================================
  Hits        28239   28239           
  Misses       1062    1062           

Flag	Coverage Δ
net48	95% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Review

Reviewed the diff (.gitignore, CodeGen/Helpers/PrefixBuilder/UnitPrefixBuilder.cs, Docs/quantity-and-unit-definition-schema.md). This is a CodeGen-only change — no unit/quantity JSON edits and no generated code diffs, so no new units/quantities to evaluate against the addition criteria.

Breaking changes

None for library consumers. This only adds a build-time guard in CodeGen, so it can only ever break generate-code.bat for future/malformed definitions, not runtime behavior. I checked every existing Prefixes-bearing unit across Common/UnitDefinitions/*.json against the new LooksLikePoweredUnit check (both the Square/Cubic name prefix and the leading-superscript abbreviation regex) — none currently trigger it, so this won't break the next generate-code.bat run for existing definitions.

Style & conventions

• Follows the established pattern of throwing UnitsNetCodeGenException for definition-validation failures (matches QuantityRelationsParser and UnitEnumValueAllocator), and the doc comment additions on GeneratePrefixUnits are a nice touch.
• The refactor from double-foreach to an outer foreach + early continue is a clean, behavior-preserving simplification.

Generated code changes

None — this PR doesn't touch generators that emit *.g.cs, so there's no generated-code diff to review for Length/Temperature/Level-style quantities.

Code quality / correctness

• LeadingPoweredUnitAbbreviationRegex (^[^\s/·*()\-\d]+(?:[²³⁴]|\^[234])) is well-targeted: it correctly matches bare powered abbreviations like m², ft³, but not compound ones like N/m², g·m², or g/mm³, since the leading run stops at / or · before reaching a superscript. Good — those compound abbreviations (pressure, moment of inertia, density) are legitimately prefixable without ambiguity, and this doesn't over-flag them. I verified this against the actual Prefixes-bearing definitions in the repo.
• Excluding digits from the leading-char class is a nice detail — it correctly avoids flagging 10³·m³ (the ThousandCubicMeter abbreviation), which starts with a digit rather than a unit letter.
• One thing worth double-checking with the author: unit.SingularName.StartsWith("Cubic") also catches compound names like CubicMeterPerKilogram or CubicMeterPerMeter, not just bare CubicMeter-style units. That appears to be intentional (prefixing CubicMeterPerKilogram would have the same kg → km³/kg-looking ambiguity as prefixing CubicMeter directly), but it's worth a one-line comment in the code confirming that's deliberate so a future reader doesn't "fix" it to be narrower.

Test coverage

There's no dedicated test project for CodeGen in this repo, so the lack of automated tests for ThrowIfPrefixesAreUnsafeForPoweredUnit/LooksLikePoweredUnit is consistent with existing convention rather than a regression. The PR description shows the author manually verified both the positive case (temporarily adding Prefixes to Area.SquareMeter, confirming the failure) and a full generate-code.bat run — reasonable given there's no harness for CodeGen unit tests currently. If CodeGen ever gets a test project, this validation logic (with its several edge cases: compound abbreviations, digit-leading abbreviations, per-locale abbreviations) would be a good first candidate to cover.

Other

• .gitignore addition (.temp/) looks like a leftover from local validation testing — harmless and reasonable to keep.

Overall a well-scoped, low-risk, backward-compatible guard that directly follows up on the KilocubicMeter/km³ ambiguity bug from #1700. Nice work tracing the regex edge cases (digits, compound units) carefully.

[claude]

Review: Fail codegen for powered unit prefixes (#1702)

Good, focused follow-up to #1700 — this closes the loop by making CodeGen fail loudly instead of relying on developers remembering the rule. Reviewed CodeGen/Helpers/PrefixBuilder/UnitPrefixBuilder.cs and the docs update.

Breaking changes
None for library consumers. This only affects CodeGen build-time validation — if it fails, it fails at code-gen time for contributors, not at runtime for users of UnitsNet.

Correctness of the heuristic
Checked the new LeadingPoweredUnitAbbreviationRegex against every Prefixes usage currently in Common/UnitDefinitions/*.json: none of the existing valid prefixed units (Volume liters/gallons, Density kg-based units, MassMomentOfInertia, VolumeFlow liters, etc.) trip the new guard, and KinematicViscosity.Stokes (which does use Prefixes) is correctly left alone. Also confirmed all existing Square*/Cubic*-named units are genuinely area/volume units, so the name-based check has no false positives today.

The regex itself is reasonably careful (anchored at ^, excludes /, ·, *, (, ), -, digits from the leading token so kg/m³, N·m², and m/s² are correctly not flagged, while ft³/s and m³ correctly are flagged via backtracking). The doc comment explaining what it deliberately does not handle (denominators, later compound terms, numeric scale factors like 10³·m³) is a nice touch for future maintainers.

One thing worth double-checking: the guard only inspects unit.Localization abbreviations and SingularName. Prefix-specific overrides via AbbreviationsForPrefixes (e.g. Density.json's "AbbreviationsForPrefixes": { "Kilo": "kip/in³" }) are not checked, but that seems fine since those are outputs derived from the prefix, not inputs the guard needs to inspect.

Style/structure
The refactor from a single nested foreach to an early-continue plus ThrowIfPrefixesAreUnsafeForPoweredUnit call before the inner prefix loop is clean and does not change behavior for the non-powered case. LooksLikePoweredUnit/ThrowIfPrefixesAreUnsafeForPoweredUnit naming and placement follow existing conventions in the file (small, well-documented private static helpers). XML doc comments are thorough and consistent with the rest of the file's style.

Minor/nit
The .gitignore addition (.temp/) looks unrelated to the PR's stated purpose (codegen guard for powered prefixes). Possibly a leftover from local validation, worth confirming it is intentional or splitting it out to keep the PR scope tight.

Test coverage
There is no CodeGen.Tests project in this repo, so this gap is not new to this PR: the whole CodeGen project currently has zero unit tests, and prior PRs in this area (e.g. #1700) validated via manual generate-code.bat runs plus downstream UnitsNet.Tests. Given that, the PR's manual validation approach (temporarily adding Prefixes to Area.SquareMeter and confirming the failure) is consistent with existing practice, but since this is regex-based logic that could regress silently, a dedicated test would be valuable if/when a CodeGen.Tests project exists. Not a blocker for this PR.

Overall
Solid, narrowly-scoped defensive change that directly prevents reintroduction of the bug fixed in #1700. No breaking changes for consumers, no new quantities/units involved. Only asks: confirm the .gitignore change is intentional, and consider a follow-up to add regression tests once CodeGen.Tests exists.

[claude]

Review

Reviewed as a follow-up to #1700: adds a CodeGen guard that fails the build when Prefixes is combined with a unit that looks like a powered unit itself (e.g. SquareMeter, CubicMeter, or an abbreviation like ft³), preventing bugs like KilocubicMeter → km³.

Breaking changes
None for library consumers — this only affects CodeGen build-time validation. It's a "breaking change" only in the sense that it would fail CI for a future PR that adds Prefixes to a powered-looking unit; it doesn't affect any currently-generated code. I checked all 90 unit definitions that currently use Prefixes (Common/UnitDefinitions/*.json) and confirmed none of them are on a Square*/Cubic*-named unit or a unit whose abbreviation starts with a powered symbol, so this won't break existing codegen.

Generated code
No generated code changes — this PR only touches the generator (CodeGen/Helpers/PrefixBuilder/UnitPrefixBuilder.cs) and docs, no Common/UnitDefinitions/*.json files changed, so there's nothing to compare in */GeneratedCode/.

Code quality

• The LeadingPoweredUnitAbbreviationRegex heuristic and its exclusions (denominator terms, compound terms after ·, leading numeric scale factors) are clearly documented in the XML remarks, and the doc comment's stated non-goals match the actual regex behavior when traced through (e.g. m/s² and kg/m³ are correctly not flagged since the power isn't on the leading token, 10³·m³ isn't flagged since it starts with a digit).
• ThrowIfPrefixesAreUnsafeForPoweredUnit is called once per unit before entering the per-prefix loop, so it's not swallowed by the existing generic catch (Exception e) in the prefix loop — the more specific UnitsNetCodeGenException message survives with useful guidance (suggests CubicKilometer/ThousandCubicMeter alternatives).
• Reuses the existing UnitsNetCodeGenException type rather than introducing a new one — consistent with the rest of CodeGen/Exceptions.
• The refactor from the stacked foreach...foreach to an explicit if (!unit.Prefixes.Any()) continue; plus nested loop is a reasonable minimal change to hook in the per-unit check without altering behavior for units without Prefixes.

Minor/nit

• The name-based check (StartsWith("Square")/StartsWith("Cubic")) is duplicated conceptually with the abbreviation-based regex check — e.g. SquareMeter's abbreviation m² would already be caught by the regex, so the name check is really just a redundant safety net for a unit whose Square/Cubic name doesn't happen to have a powered-looking abbreviation. Worth a one-line comment on LooksLikePoweredUnit calling this out (it's already implied by the <remarks> on the regex field, but not stated at the call site), or is intentional and could be left as-is — the redundancy is cheap and arguably good defense-in-depth.
• .gitignore adds .temp/ — unrelated to the core change but harmless (likely left over from the manual validation steps described in the PR body).

Test coverage
No automated test exercises the new guard (there's no existing CodeGen test project in the repo to add one to, so this is consistent with current conventions). The PR description states manual validation was done by temporarily adding Prefixes to Area.SquareMeter and confirming the CodeGen failure, then reverting — that's a reasonable substitute given there's no CodeGen test harness, but it does mean regressions in this heuristic wouldn't be caught by CI going forward.

Security
None.

Overall this is a small, well-targeted, well-documented safety net with no impact on currently generated code. Nice follow-up to #1700.

[claude]

Review

Reviewed the diff (.gitignore, CodeGen/Helpers/PrefixBuilder/UnitPrefixBuilder.cs, Docs/quantity-and-unit-definition-schema.md). This is a CodeGen-only change (no GeneratedCode/*.g.cs or unit JSON files touched), so no breaking changes to the public API.

Breaking changes

None. This only affects code generation; it fails the build if a future JSON definition misuses Prefixes on a powered unit. No existing generated types change.

Regression risk (no existing units break)

I sampled several quantities that combine Prefixes with compound/powered abbreviations to check LooksLikePoweredUnit doesn't produce false positives:

• MassMomentOfInertia.GramSquareMeter (Prefixes: [Milli, Kilo], abbreviation g·m²) — correctly not flagged, because the regex only inspects the token before the first ·// etc., so it sees g, not m². This is the important case the heuristic is designed around (prefix applies to gram, not to the squared meter).
• SpecificWeight.NewtonPerCubicMeter (N/m³), PowerDensity.WattPerCubicMeter (W/m³), EnergyDensity.JoulePerCubicMeter (J/m³), VolumetricHeatCapacity (J/(m³·K)) — all correctly skipped since the cubed unit is in the denominator.
• Volume.Liter, ImperialGallon, UsGallon (the only Prefixes users in Volume.json) — none are powered, unaffected. CubicMeter, CubicKilometer, ThousandCubicMeter etc. are already explicit units without Prefixes, consistent with the doc update.

I didn't find a case among the ~90 JSON files using Prefixes that would newly trip this guard, which matches the PR description's manual validation. Nice, focused heuristic.

Code quality / potential gap

LooksLikePoweredUnit only inspects the unit's own base Abbreviations, not AbbreviationsForPrefixes overrides (e.g. Density.PoundPerCubicInch uses "AbbreviationsForPrefixes": { "Kilo": "kip/in³" }). None of the current overrides in the repo are problematic, but in principle a unit could have a safe base abbreviation while defining a per-prefix override abbreviation that is a leading powered form — that path wouldn't be checked. Probably fine to leave out of scope given how narrowly this heuristic is intended, but worth a one-line doc/remarks callout if intentional.

Minor style nit: ThrowIfPrefixesAreUnsafeForPoweredUnit reads a little awkwardly; something like ThrowIfUnitLooksLikePoweredUnit might read more naturally, but this is very much a nit.

The refactor from the flat foreach unit / foreach prefix to an early continue when !unit.Prefixes.Any() is a good change — it ensures the new guard only triggers for units that actually opt into Prefixes, so Area.SquareMeter/Volume.CubicMeter (which never set Prefixes) aren't retroactively broken.

Test coverage

There's no CodeGen.Tests project in this repo, so this (like the neighboring UnitEnumValueAllocator validation) is only exercised by manually running generate-code.bat — CI's build.ps1 doesn't invoke CodeGen to re-validate JSON definitions. That's an existing architectural gap rather than something introduced here, but it does mean a regex edge case regression could slip in silently until someone runs codegen locally. Not blocking, just noting it since the PR touches validation logic specifically.

Docs

The schema doc update is clear and gives good concrete examples (km²/km³ vs Watt/Joule/Ohm). Nicely explains why the heuristic is naming-based rather than dimension-based.

Summary

Solid, narrowly-scoped guard that matches its stated goal, with good regex documentation (remarks explicitly list what it does not catch, which is honest about the heuristic's limits). No breaking changes, no new units. Only real suggestion is considering whether AbbreviationsForPrefixes overrides should also be checked, and it'd be nice to eventually get a lightweight CodeGen unit test project so heuristics like this have regression coverage beyond manual verification.

[claude]

Review of #1702: Fail codegen for powered unit prefixes

Nice, focused change: it closes a real gap (e.g. accidentally regenerating something like km-cubed for CubicMeter) with a narrowly scoped heuristic rather than a full unit-expression parser. A few notes.

Breaking changes:
None. This only affects CodeGen (a build-time tool) and adds a new failure mode for future JSON definitions that misuse Prefixes on powered units. No files under GeneratedCode are touched in this diff, and the PR description confirms generate-code.bat produced no changes, so no existing quantity or unit is affected today.

Generated code changes:
None. This PR only touches CodeGen/Helpers/PrefixBuilder/UnitPrefixBuilder.cs and docs. Worth calling out explicitly since the diff has zero impact on Length, Temperature, Level, or any other generated quantity.

New quantities/units:
None added.

Code quality / correctness:

• The LooksLikePoweredUnit heuristic is well documented, and its exclusions (denominators like kg/m-cubed, later compound terms like N times m-squared, leading numeric scale factors like 1000 times m-cubed) are called out explicitly in the XML doc. That avoids false positives on legitimate compound units such as HeatTransferCoefficient.CaloriePerHourSquareMeterDegreeCelsius (abbreviation kcal/(hm^2C)), which I confirmed is not flagged because the leading token "kcal" is not itself powered.
• I spot-checked the existing Common/UnitDefinitions JSON files with Prefixes set against the new regex/name check (e.g. DoseAreaProduct.GraySquareDecimeter, abbreviation Gy*dm^2) and did not find any that would newly trip the guard, consistent with the "no generated code diff" claim in the PR description.
• Minor: the caret-power regex fragment will also match inside something like m^24 (matches the ^2 substring), but that is not a realistic abbreviation format in this codebase, so it is not a practical concern.
• Minor: ThrowIfPrefixesAreUnsafeForPoweredUnit throws UnitsNetCodeGenException directly, outside the existing try/catch that wraps prefix errors in a generic Exception with unit/prefix context. That is fine since the new exception message already includes the quantity and unit name; just flagging the slightly inconsistent error-wrapping between the two failure paths in this method.
• Unrelated: the .gitignore addition of .temp/ looks like a leftover from local validation rather than part of the core change. Harmless, but might be worth splitting into its own PR or explaining in the description.

Test coverage:
There is no dedicated CodeGen unit test project in the repo today, so the manual validation described in the PR (temporarily adding Prefixes to Area.SquareMeter and confirming the failure) is consistent with existing practice. That said, this is exactly the kind of regex-heuristic logic (leading-token detection, several documented exclusions) that tends to accumulate edge cases over time. If a CodeGen test project is added in the future, LooksLikePoweredUnit and the leading-powered-unit regex would be a good first candidate for direct unit tests rather than relying on full-repo codegen runs.

Performance / security:
No concerns. This runs once at build time over roughly 131 JSON files, and there is no security-sensitive surface here.

Overall: solid, well-documented guardrail that matches the pattern established in #1700. Nothing blocking.