Skip to content

[patch] Keep Tempo.TryParse from throwing, and reject infinite tempos and non-positive beats - #401

Merged
matt-edmondson merged 1 commit into
mainfrom
fix/305-tempo-validation
Oct 9, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
fix/305-tempo-validation

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #305

What was wrong

Tempo had two validation gaps:

  • TryParse could throw. Its guard was bpm <= 0.0, which is false for NaN, so "NaNbpm@1/4" reached Create, and Create threw ArgumentOutOfRangeException. That breaks the Try-pattern contract for untrusted text.
  • Values that make Seconds meaningless were accepted. An infinite tempo, and a zero or negative beat (0/4, -1/4), passed both TryParse and Create. Seconds divides by the beat, so these gave Infinity or negative time.

Change (Semantics.Music/Tempo.cs)

  • Two private predicates hold the rules: IsValidBeatsPerMinute (positive and finite) and IsValidBeat (Numerator > 0; Duration normalises the sign onto the numerator, so 1/-4 is caught too).
  • Create throws ArgumentOutOfRangeException for a non-finite or non-positive tempo, and for a non-positive beat (after the existing null check).
  • TryParse applies the same predicates itself, so it returns false for these inputs and never reaches a throwing Create.
  • Updated the XML docs to match.

The triage also mentioned Duration.TryParse accepting 0/4 and -1/4. I left Duration alone because zero and negative durations may be legitimate for other consumers (for example, differences). This PR only stops Tempo from using one as its beat.

Tests (TempoTests)

  • TryParseReturnsFalseForInvalidTempo: NaN, -NaN, ±Infinity, 0, and -120 bpm, plus 0/4, -1/4 and 1/-4 beats. Each returns false with a null result.
  • TryParseAcceptsValidTempo: "120bpm@1/4" still parses and round-trips.
  • CreateThrowsForInvalidBeatsPerMinute: NaN, ±Infinity, 0 and -1.
  • CreateThrowsForNonPositiveBeat: 0/1, 0/4, -1/4 and 1/-4.

With Tempo.cs reverted, 11 of these cases fail. With the fix, the full Semantics.Test suite passes: 1499 passed, 0 failed, 8 skipped. Semantics.Music builds for every target framework.

🤖 Generated with Claude Code

https://claude.ai/code/session_019RYN9jT6szUiySkywFKCA1


Generated by Claude Code

… and non-positive beats

Fixes #305

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RYN9jT6szUiySkywFKCA1
@sonarqubecloud

sonarqubecloud Bot commented Oct 8, 2026

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit fe711e3 into main Oct 9, 2026
15 checks passed
@matt-edmondson
matt-edmondson deleted the fix/305-tempo-validation branch October 9, 2026 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants