Skip to content

Commit 6889f5b

Browse files
authored
fix(sonar): move the rule exemptions to where they take effect (#881)
Closes #880. ## What the issue asked, and what turned out to be true #880 offered two ways out: withdraw `e1`/`e2`, or move them where they would work. Investigating which showed a third fact that settles it — **the project settings already carried a `sonar.issue.ignore.multicriteria` entry**, and it had been working the whole time: ```json {"resourceKey": "**/Tests/**", "ruleKey": "php:S1313"} ``` So the mechanism is not unavailable. Only its location was wrong. Withdrawing the two entries would have thrown away a decision someone made on purpose; moving them keeps it and makes it real. ## The change `e1` and `e2` moved verbatim into the project settings, with the pre-existing entry carried through rather than overwritten: ```json {"resourceKey": "**/Tests/**", "ruleKey": "php:S1313"} {"resourceKey": "Classes/Domain/Model/**", "ruleKey": "php:S1448"} {"resourceKey": "Tests/**", "ruleKey": "php:S5332"} ``` Read back after writing, because a `204` says the request was accepted, not that the field landed. The block in `.sonarcloud.properties` is replaced by a comment that says where the exemptions live, how to read them back, and — the part that matters for the next person — **which kind of finding belongs where**: - a **one-off** intentional finding is marked *Won't Fix* on the issue itself, with the reason as a comment; - a **rule-level** exemption, which will fire again on a file nobody has written yet, goes into settings. What stays in the file is what the file can actually do: `sonar.exclusions` and `sonar.cpd.exclusions`. Both are honoured — adding `Resources/Private/Demo/**` to the second one in #879 removed a 34.8% duplication condition from that pull request's gate, which is the measurement this whole distinction rests on. ## Why the block was worse than nothing It read as an applied decision. `e2` exempted `php:S5332` under `Tests/` and its comment named `Tests/Unit/Service/Skill/GitHubClientTest.php` as the reason — and that exact file kept reporting that exact rule on `main`. Anyone reading the file would have concluded the rule was handled. #879 came within one commit of adding three more entries to it, on the same assumption. They were withdrawn only because they were tested. _Assisted by claude-code:claude-opus-5 — [Session](https://claude.ai/code/session_01MNg1MysJVugv1xo2husknU)_
2 parents 0b9bc54 + 5183f5a commit 6889f5b

1 file changed

Lines changed: 23 additions & 23 deletions

File tree

.sonarcloud.properties

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -35,29 +35,29 @@ sonar.cpd.exclusions=Resources/Public/JavaScript/Vendor/**,Resources/Private/Tem
3535
# column is the framework's mapping convention, not a design smell.
3636
# - e2 (php:S5332, "clear-text protocol"): GitHubClientTest deliberately uses an
3737
# http:// URL to assert the client REJECTS it (HostNotAllowedException).
38-
# ISSUE-LEVEL IGNORES DO NOT WORK HERE — measured, not assumed.
38+
# ISSUE-LEVEL IGNORES DO NOT LIVE HERE — they live in the project settings.
3939
#
40-
# Automatic Analysis supports only a subset of sonar.* properties from this
41-
# file. sonar.exclusions and sonar.cpd.exclusions ARE honoured (removing the
42-
# duplication finding on Resources/Private/Demo/** from this file is what
43-
# proved it). sonar.issue.ignore.multicriteria is NOT: e2 below exempts
44-
# php:S5332 under Tests/, and the exact file its comment names —
45-
# Tests/Unit/Service/Skill/GitHubClientTest.php — still reports that rule on
46-
# main today. e1 is inert for the same reason. Both have been carried here
47-
# since they were written, doing nothing.
40+
# Automatic Analysis honours only a subset of sonar.* properties from this
41+
# file. sonar.exclusions and sonar.cpd.exclusions ARE honoured; the duplication
42+
# exclusion for Resources/Private/Demo/** above is what proved it, by removing
43+
# that condition from a pull request's quality gate.
44+
# sonar.issue.ignore.multicriteria is NOT.
4845
#
49-
# They are left in place rather than deleted, because removing another
50-
# decision's record is not this change's business — that is #880 — but nothing
51-
# new should be added to the block, and nothing should be assumed exempt
52-
# because it appears here. An intentional finding is marked in SonarCloud itself (Won't Fix, with
53-
# the reason as a comment on the issue), which does work.
46+
# Two entries used to sit here and did nothing. e2 exempted php:S5332 under
47+
# Tests/ and its comment named Tests/Unit/Service/Skill/GitHubClientTest.php as
48+
# the reason; that file kept reporting that rule. e1 (php:S1448 under
49+
# Classes/Domain/Model/**) was inert the same way.
5450
#
55-
# Marked that way for this change: php:S5332 on the demo Ollama endpoint (a
56-
# container-internal host with no TLS listener) and php:S2003 x3 ("use
57-
# require_once"), where the rule's fix breaks all three call sites — two read a
58-
# file that RETURNS the demo array, and the third must re-run per request.
59-
sonar.issue.ignore.multicriteria=e1,e2
60-
sonar.issue.ignore.multicriteria.e1.ruleKey=php:S1448
61-
sonar.issue.ignore.multicriteria.e1.resourceKey=Classes/Domain/Model/**
62-
sonar.issue.ignore.multicriteria.e2.ruleKey=php:S5332
63-
sonar.issue.ignore.multicriteria.e2.resourceKey=Tests/**
51+
# Both are now in SonarCloud's project settings — Administration › General
52+
# Settings › Analysis Scope › Issues — where a third entry (php:S1313 under
53+
# **/Tests/**) had been living correctly all along, which is what showed the
54+
# mechanism works and only its location was wrong. Read them back with:
55+
#
56+
# curl -H "Authorization: Bearer $SONAR_TOKEN" \
57+
# 'https://sonarcloud.io/api/settings/values?component=netresearch_t3x-nr-llm&keys=sonar.issue.ignore.multicriteria'
58+
#
59+
# A one-off intentional finding does not need an entry there at all: mark the
60+
# issue Won't Fix in SonarCloud with the reason as a comment. A rule-level
61+
# exemption, which will fire again on a file that does not exist yet, does.
62+
#
63+
# See #880.

0 commit comments

Comments
 (0)