Skip to content

Fix --fmt with --by-percent producing zeros and warnings (#967)#971

Merged
AlDanial merged 4 commits into
AlDanial:masterfrom
AnasImloul:fix/issue-967-by-percent-fmt
May 9, 2026
Merged

Fix --fmt with --by-percent producing zeros and warnings (#967)#971
AlDanial merged 4 commits into
AlDanial:masterfrom
AnasImloul:fix/issue-967-by-percent-fmt

Conversation

@AnasImloul

Copy link
Copy Markdown
Contributor

Summary

Fixes #967.

When --fmt is combined with --by-percent, cloc round-trips its results through an intermediate JSON. That JSON uses blank_pct / comment_pct / code_pct keys, but print_format_n only reads the canonical blank / comment / code. The result was a stream of Use of uninitialized value warnings on STDERR and a table of zeros.

Repro (before this PR)

$ cloc --fmt=4 --by-percent=cm <some dir>
Use of uninitialized value $nB in addition (+) at .../cloc line 15819.
Use of uninitialized value $nCm in addition (+) at .../cloc line 15820.
... (many more)
File      blank  comment     code    Total
./a.c         0        0        0        0
./b.py        0        0        0        0

After

File      blank  comment     code    Total
./a.c      0.00    33.33    66.67      100
./b.py     0.00    33.33    66.67      100

Changes

  • cloc — in load_json:
    • Alias *_pct keys to their canonical names so the rest of the format pipeline works unchanged.
    • Fall back from a missing language field to the entry key (by-language reports don't carry one), avoiding a separate length(undef) warning.
  • Unix/t/01_opts.t — regression test that asserts no Use of uninitialized value on STDERR and non-zero output for --fmt=4 --by-percent=cm.

Test plan

  • Original repro from the issue produces clean output with no warnings.
  • prove t/00_C.t t/01_opts.t t/02_git.t — all 952 tests pass (including 2 new).

When --fmt is combined with --by-percent, the intermediate JSON uses
blank_pct/comment_pct/code_pct keys, but print_format_n only reads
the canonical blank/comment/code keys. The result was a flood of
"Use of uninitialized value" warnings and a table of zeros.

In load_json, alias *_pct keys to their canonical names so downstream
formatting works unchanged. Also fall back from a missing 'language'
field to the entry key (by-language reports don't carry one).

Add a regression test under Unix/t/01_opts.t that asserts no
"uninitialized value" warnings and non-zero output for
--fmt=4 --by-percent=cm.
@AlDanial

AlDanial commented May 7, 2026

Copy link
Copy Markdown
Owner
/tmp/cloc » ./cloc --fmt 3 --by-percent=cm tests/inputs            
Illegal division by zero at ./cloc line 4892.

/tmp/cloc » ./cloc --fmt 4 --by-percent=cm tests/inputs
Illegal division by zero at ./cloc line 4892.

AnasImloul added 2 commits May 7, 2026 09:43
Maintainer pointed out that --fmt 3/4 --by-percent=cm tests/inputs
crashes with "Illegal division by zero at cloc line 4892". This is
a pre-existing defect on master, distinct from the JSON-key
mismatch fixed in the previous commit, but in the same code path:
generate_report computes the txt $data_line for every output style
(then discards it for non-txt), so the division at line 4892 fires
even when the active output is JSON. tests/inputs/issues/644/UInt8.cs
has code=0, comment=0, blank=1, making $DEN=0 under --by-percent=cm.

Add the same > 0 guards already used in diff_report (lines 3725, 4076)
to the txt, yaml/json, csv/md, and SUM branches in generate_report.
When the denominator is zero, print 0.00 instead of crashing.

Extend the regression test to assert no "Illegal division by zero"
on the full tests/inputs corpus for --fmt=3 and --fmt=4 with
--by-percent=cm.
The earlier test only checked for absence of error strings on STDERR.
Add a positive assertion that the empty-content file
tests/inputs/issues/644/UInt8.cs (code=0, comment=0, blank=1) shows
0.00 across all three percentage columns. This catches silent
regressions where the values become empty, NaN-like, or otherwise
malformed without producing a "division by zero" message.
@AnasImloul

Copy link
Copy Markdown
Contributor Author

@AlDanial Thanks for surfacing those failing modes. I verified that the Illegal division by zero at cloc line 4892 is pre-existing on master (i.e. not introduced by my PR, it reproduces against master with tests/inputs/issues/644/UInt8.cs, which has code=0, comment=0, blank=1, making $DEN = 0 under --by-percent=cm).

The reason it surfaces under --fmt 3/4 is that generate_report computes the txt $data_line for every output style, even when the active output is JSON, so the division at line 4892 fires before the --fmt consumer ever sees the JSON.

Since it's in the same code path as #967, I've extended this PR to fix it too:

  • Added > 0 guards (matching the existing diff_report pattern at lines 3725, 4076) to the txt, yaml/json, csv/md, and SUM branches in generate_report. When the denominator is zero, the field is emitted as 0.00 rather than crashing.
  • Extended the regression test in Unix/t/01_opts.t to run --fmt=3 --by-percent=cm and --fmt=4 --by-percent=cm against the full tests/inputs corpus and assert no Illegal division by zero on STDERR.

Verified clean against:

  • --fmt 3/4 --by-percent=cm tests/inputs.
  • --by-percent=cm tests/inputs (no --fmt), --xml, --csv, and --by-file variants.
  • prove t/00_C.t t/01_opts.t t/02_git.t.

@AlDanial

AlDanial commented May 8, 2026

Copy link
Copy Markdown
Owner

This is good progress.

Couple of minor issues: 1) the SUM line for all --fmt values and 2) the Total column for --fmt 2, 4, and 5 are incorrect.

Per maintainer review: with --fmt and --by-percent, the SUM line was
the arithmetic sum of per-row percentages, which is meaningless
(e.g. "SUM 6970.42 11787.25 31212.75" for --fmt 4 --by-percent=cm).

Preserve the JSON's SUM block in load_json (previously deleted) and
pass it to print_format_n. When --by-percent is active, replace the
accumulated SUM cells with the aggregate from that block:
- 't' mode: the JSON already stores 100/100/100, use directly.
- 'c'/'cm'/'cb'/'cmb': the JSON stores raw counts; convert to
  aggregate percentages via compute_denominator(), the same logic
  the non-fmt text path uses.

The Total column on the SUM row is then the sum of the corrected
aggregates (e.g. 115.87 for --by-percent=cm), matching the layout of
count mode where Total = blank+comment+code per row.

Per-row Total in --fmt 2/4/5 under --by-percent is left as the
sum of the row's percentages (open question for the maintainer:
the JSON does not carry raw per-row counts in by-percent mode, so
showing real line totals would require a JSON shape change).

Add a regression test that compares the SUM row produced by --fmt 1-5
against the SUM row of the non-fmt text path (the source of truth)
for --by-percent=cm on tests/inputs.
@AlDanial
AlDanial merged commit 72d8520 into AlDanial:master May 9, 2026
1 check passed
@AlDanial

AlDanial commented May 9, 2026

Copy link
Copy Markdown
Owner

thank you!

@AnasImloul
AnasImloul deleted the fix/issue-967-by-percent-fmt branch May 10, 2026 18:50
AlDanial added a commit that referenced this pull request Jun 21, 2026
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 this pull request may close these issues.

when using --by-percent: "Use of uninitialized value"

2 participants