Move Content Type header values to a separate class
#47
Conversation
This will make it easier to use those in projects where `SecurityTxt` as a name is already a thing, to avoid at least some conflicts.
There was a problem hiding this comment.
Pull request overview
This PR refactors Content-Type header constants by moving them from the SecurityTxt class to a new dedicated SecurityTxtContentType class. This separation helps avoid naming conflicts in projects where SecurityTxt is already used. The PR also improves naming clarity by distinguishing between "charset" (the value utf-8) and "charset parameter" (the full parameter string charset=utf-8), aligning with RFC 9110 terminology.
Key changes:
- Created new
SecurityTxtContentTypeclass with refined constants:CONTENT_TYPE,CHARSET,CHARSET_PARAMETER, andMEDIA_TYPE - Renamed internal
SecurityTxtContentTypeclass toSecurityTxtFetchHostContentTypeto avoid naming conflicts - Updated all references throughout the codebase to use the new class and improved naming
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/SecurityTxtContentType.php | New class containing Content-Type related constants with improved naming (CHARSET vs CHARSET_PARAMETER) |
| src/SecurityTxt.php | Removed Content-Type constants now hosted in dedicated class |
| src/Fetcher/SecurityTxtFetchHostContentType.php | Renamed from SecurityTxtContentType; updated property and method names from "charset" to "charsetParameter" |
| src/Fetcher/SecurityTxtFetcherFetchHostResult.php | Updated type hints and instantiation to use renamed SecurityTxtFetchHostContentType class |
| src/Fetcher/SecurityTxtFetcher.php | Updated imports and references to use SecurityTxtContentType constants and renamed getter methods |
| src/Violations/SecurityTxtContentTypeInvalid.php | Updated imports and constant references to use new SecurityTxtContentType class |
| src/Violations/SecurityTxtContentTypeWrongCharset.php | Updated imports, parameter names, and constant references to align with new naming conventions |
| tests/Fetcher/SecurityTxtFetcherTest.phpt | Updated test imports and constant references to use new SecurityTxtContentType class |
| tests/Fetcher/SecurityTxtFetcherFetchHostResultTest.phpt | Updated test to call renamed getLowercaseCharsetParameter() method |
| README.md | Updated documentation to reflect new constant names and added CHARSET_PARAMETER to the list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Interesting thing spotted in the agent session log:
Too bad it cannot comment on it, because that would be a valid comment. Changed in the following commit. |
"Charset" is just `utf-8`, while "charset parameter" is the whole string `charset=utf-8`.
It's not like these would change, ever. Also helps in IDEs when they show the constant value on hover for example, then you'd like to see the actual value, not what it's concatenated of. Follow-up to #47
It's not like these would change, ever. Also helps in IDEs when they show the constant value on hover for example, then you'd like to see the actual value, not what it's concatenated of. Follow-up to #47
This will make it easier to use those in projects where
SecurityTxtas a name is already a thing, to avoid at least some conflicts.Better naming as a bonus: add "parameter" suffix to "charset" where it makes sense:
"Charset" is just
utf-8, while "charset parameter" is the whole stringcharset=utf-8as per RFC 9110.Follow-ups: