diff --git a/.specify/feature.json b/.specify/feature.json deleted file mode 100644 index 48bae1d..0000000 --- a/.specify/feature.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "feature_directory": "specs/001-packagist-release-automation" -} diff --git a/.specify/memory/constitution.md b/.specify/memory/constitution.md deleted file mode 100644 index 9db590b..0000000 --- a/.specify/memory/constitution.md +++ /dev/null @@ -1,158 +0,0 @@ - - -# Project Constitution — knplabs/phpstan-rules - -**Constitution Version:** 1.0.0 -**Ratification Date:** 2026-07-15 -**Last Amended Date:** 2026-07-15 -**Maintainers:** KnpLabs organization - ---- - -## Project Identity - -**Name:** knplabs/phpstan-rules -**Package type:** PHPStan extension (open-source, MIT) -**Purpose:** Provide PHPStan static-analysis rules shared across KnpLabs organization projects, -made available to the wider PHP community as a Composer package. -**Primary audience:** PHP developers who want to enforce organization-wide coding conventions via -PHPStan. - ---- - -## Core Principles - -### Principle 1 — Broad Compatibility - -Every release MUST remain installable on the widest reasonable range of PHP and PHPStan versions -to maximize adoption. Dropping a supported version is a MAJOR version bump and MUST be explicitly -justified. - -**Non-negotiable rules:** -- `composer.json` `require.php` MUST target `^8.2` or broader (never pin to a single patch). -- `composer.json` `require.phpstan/phpstan` MUST support at least the current stable major series - and, where API-compatible, the previous major. -- CI MUST gate on every declared PHP minor version (currently 8.2, 8.3, 8.4, 8.5). -- No rule implementation MUST use PHP APIs unavailable in the declared minimum version. -- Dependencies MUST be kept minimal; new `require` entries need explicit justification. - -**Rationale:** Restrictive version constraints are the primary reason PHPStan extensions go -un-adopted. A wide compatibility matrix lowers the barrier to entry and encourages community -contributions. - ---- - -### Principle 2 — Test Coverage per Rule - -Every PHPStan rule MUST ship with a dedicated `PHPUnit` / `PHPStan\Testing\RuleTestCase` test -covering both triggering and non-triggering cases. - -**Non-negotiable rules:** -- Each rule class in `src/Rules//` MUST have a corresponding test in - `tests/Rules//` following the naming convention `Test.php`. -- Each test MUST include at least one fixture file in `tests/Rules//fixtures/` that - exercises both the positive (violation) and negative (no violation) paths. -- New rules MUST NOT be registered in `extension.neon` until their test passes on all declared - PHP versions. -- CI MUST run the full test suite (`composer test`) and it MUST be green before merging. - -**Rationale:** Rules without tests are liabilities — they cannot be refactored safely and may -produce false positives or negatives undetected across PHPStan/PHP upgrades. - ---- - -### Principle 3 — Documentation per Rule - -Every PHPStan rule MUST be documented in `README.md` before it is released. - -**Non-negotiable rules:** -- Each rule entry in `README.md` MUST include: - - The rule identifier (e.g., `clock.disallowDateTimeNow`). - - A one-sentence summary of what the rule enforces and why. - - A "Triggers on" code block with at least one violating example. - - A "Does not trigger on" code block with at least one passing example. - - A "Recommended fix" section showing the preferred alternative pattern. -- Documentation MUST be kept in sync with rule behavior; a PR that changes a rule's logic - MUST also update its documentation. -- Rules that are not yet documented MUST NOT appear in a stable release. - -**Rationale:** Without clear documentation, developers cannot evaluate whether a rule is -appropriate for their project, and they cannot understand why a violation was flagged. - ---- - -### Principle 4 — Open-Source Quality Standards - -All source files MUST meet the project's code-quality baseline so that community contributors -can work effectively. - -**Non-negotiable rules:** -- All PHP source files MUST declare `strict_types=1`. -- Code MUST conform to PSR-12 enforced by `php-cs-fixer` (`composer cs-check` MUST pass). -- The codebase itself MUST pass PHPStan analysis at the configured level (`composer phpstan` - MUST pass). -- PHPStan baseline (`phpstan-baseline.neon`) MUST be kept as small as possible; new suppressions - MUST be justified in the PR description. -- Rule classes MUST be `final readonly` where PHP version and design allow. - -**Rationale:** Consistent code style and static analysis lower the cognitive load for contributors -and prevent quality regressions from being introduced silently. - ---- - -### Principle 5 — Single-Responsibility Rule Design - -Each rule class MUST enforce exactly one logical constraint. - -**Non-negotiable rules:** -- A rule MUST be scoped to a single AST node type (one `getNodeType()` return value per class). -- If two related constraints share an AST node, they MUST be implemented as separate classes - unless their logic is inseparably coupled (requires explicit justification in the PR). -- Rule namespaces (`src/Rules//`) MUST group rules by domain, not by node type. -- The rule's error message MUST be actionable: it MUST state what to do instead, not only - what was found wrong. - -**Rationale:** Small, focused rules are easier to test, easier to selectively disable in -downstream projects, and easier for contributors to understand and maintain. - ---- - -## Governance - -### Amendment Procedure - -1. Open a GitHub issue or PR describing the proposed change and its rationale. -2. At least one maintainer MUST review and approve the change. -3. The `CONSTITUTION_VERSION` MUST be bumped according to the semantic rules below. -4. `LAST_AMENDED_DATE` MUST be updated to the merge date. -5. The Sync Impact Report (HTML comment at top of this file) MUST be refreshed. - -### Versioning Policy - -- **MAJOR** bump: backward-incompatible governance change — removing a principle, redefining - a non-negotiable rule in a stricter or incompatible way. -- **MINOR** bump: new principle added, new mandatory section introduced, materially expanded - guidance. -- **PATCH** bump: clarification, wording improvement, typo fix, non-semantic refinement. - -### Compliance Review - -- Constitution compliance MUST be assessed during each PR review. -- Each speckit feature cycle (specify → plan → tasks → implement) MUST reference the active - constitution version. -- If a task or implementation choice would violate a principle, the violation MUST be escalated - before merging — not suppressed silently. diff --git a/.specify/templates/plan-template.md b/.specify/templates/plan-template.md deleted file mode 100644 index bfdf414..0000000 --- a/.specify/templates/plan-template.md +++ /dev/null @@ -1,60 +0,0 @@ -# Implementation Plan — [FEATURE_NAME] - -**Constitution version:** 1.0.0 -**Spec reference:** `.specify/specs/[feature-slug]/spec.md` -**Author:** [AUTHOR] -**Date:** [DATE] - ---- - -## Constitution Check - -Before finalizing this plan, confirm compliance with the active constitution: - -| Principle | Check | -|-----------|-------| -| 1 — Broad Compatibility | [ ] No new PHP/PHPStan version constraint tightening | -| 2 — Test Coverage per Rule | [ ] Test class and fixture planned | -| 3 — Documentation per Rule | [ ] README section planned | -| 4 — Open-Source Quality Standards | [ ] strict_types, PSR-12, PHPStan passing | -| 5 — Single-Responsibility Rule Design | [ ] One node type per class confirmed | - ---- - -## Approach - -[Describe the chosen implementation approach and why it was selected over alternatives.] - -### Alternatives considered - -| Alternative | Rejected because | -|-------------|-----------------| -| [option] | [reason] | - ---- - -## File Inventory - -| File | Action | Notes | -|------|--------|-------| -| `src/Rules/[Namespace]/[RuleName].php` | Create | Core rule class | -| `tests/Rules/[Namespace]/[RuleName]Test.php` | Create | PHPUnit test | -| `tests/Rules/[Namespace]/fixtures/[name].php` | Create | Test fixture | -| `extension.neon` | Edit | Register rule service | -| `README.md` | Edit | Add rule documentation | - ---- - -## Edge Cases & Risks - -- [List known edge cases the implementation must handle] -- [List risks, e.g., PHPStan API instability, PHP version quirks] - ---- - -## Validation Steps - -1. `docker compose run --rm php vendor/bin/phpunit tests/Rules/[Namespace]/[RuleName]Test.php` -2. `docker compose run --rm php composer phpstan` -3. `docker compose run --rm php composer cs-check` -4. Manual review of fixture file to confirm positive/negative cases. diff --git a/.specify/templates/spec-template.md b/.specify/templates/spec-template.md deleted file mode 100644 index b6bf747..0000000 --- a/.specify/templates/spec-template.md +++ /dev/null @@ -1,75 +0,0 @@ -# Feature Specification — [FEATURE_NAME] - -**Constitution version:** 1.0.0 -**Status:** [draft | review | approved] -**Author:** [AUTHOR] -**Date:** [DATE] - ---- - -## Overview - -[One paragraph describing the feature, the problem it solves, and why it belongs in this package.] - ---- - -## Scope - -### In scope -- [What this feature covers] - -### Out of scope -- [What this feature explicitly does not cover] - ---- - -## Rule Definition (if applicable) - -**Rule identifier:** `[namespace.ruleName]` -**Node type:** `[PhpParser\Node\...]` -**Namespace / directory:** `src/Rules/[Namespace]/` - -### Triggers on -```php -// [example code that should produce a violation] -``` - -### Does not trigger on -```php -// [example code that should NOT produce a violation] -``` - -### Error message -> [Exact error message string, must be actionable] - -### Recommended fix -[Describe the pattern the developer should use instead.] - ---- - -## Compatibility Requirements - - -- Minimum PHP version affected: [e.g., 8.2+] -- PHPStan version requirements: [e.g., ^2.0] -- Any new `composer.json` dependencies: [none | list with justification] - ---- - -## Acceptance Criteria - - -- [ ] Rule class exists at `src/Rules/[Namespace]/[RuleName].php` and is `final readonly`. -- [ ] Rule is registered in `extension.neon`. -- [ ] Test class exists at `tests/Rules/[Namespace]/[RuleName]Test.php`. -- [ ] Fixture file covers at least one triggering and one non-triggering case. -- [ ] `composer test` passes on PHP 8.2, 8.3, 8.4, 8.5. -- [ ] `composer phpstan` passes with no new baseline suppressions (or suppressions justified). -- [ ] `composer cs-check` passes. -- [ ] `README.md` updated with rule documentation (identifier, summary, examples, fix). - ---- - -## Open Questions - -- [List any unresolved design or scope questions] diff --git a/.specify/templates/tasks-template.md b/.specify/templates/tasks-template.md deleted file mode 100644 index a6667d3..0000000 --- a/.specify/templates/tasks-template.md +++ /dev/null @@ -1,59 +0,0 @@ -# Tasks — [FEATURE_NAME] - -**Constitution version:** 1.0.0 -**Plan reference:** `.specify/specs/[feature-slug]/plan.md` -**Author:** [AUTHOR] -**Date:** [DATE] - ---- - -## Task List - -Tasks are ordered by dependency. Mark `[x]` when complete. - -### Phase 1 — Rule Implementation - -- [ ] **T1** — Create rule class `src/Rules/[Namespace]/[RuleName].php` - - Implement `Rule<[NodeType]>` - - Add `getNodeType()` returning `[NodeType]::class` - - Add `processNode()` with violation detection logic - - Ensure `final readonly` class modifier - - Add `declare(strict_types=1)` - -- [ ] **T2** — Register rule in `extension.neon` - - Add service entry with `phpstan.rules.rule` tag - -### Phase 2 — Testing (Constitution Principle 2) - -- [ ] **T3** — Create fixture file `tests/Rules/[Namespace]/fixtures/[name].php` - - Include at least one triggering case (with inline comment marking expected violation) - - Include at least one non-triggering case - -- [ ] **T4** — Create test class `tests/Rules/[Namespace]/[RuleName]Test.php` - - Extend `PHPStan\Testing\RuleTestCase<[RuleName]>` - - Implement `getRule()` returning a new instance of the rule - - Add `testRule()` calling `$this->analyse()` with fixture path and expected errors - -- [ ] **T5** — Verify test suite passes - - `docker compose run --rm php composer test` — green on all PHP versions - -### Phase 3 — Documentation (Constitution Principle 3) - -- [ ] **T6** — Update `README.md` - - Add rule section with identifier, summary, "Triggers on", "Does not trigger on", - and "Recommended fix" - -### Phase 4 — Quality Gate (Constitution Principle 4) - -- [ ] **T7** — Verify static analysis - - `docker compose run --rm php composer phpstan` — passes, no new baseline entries - -- [ ] **T8** — Verify code style - - `docker compose run --rm php composer cs-check` — passes - ---- - -## Definition of Done - -All tasks T1–T8 checked AND a PR reviewer has confirmed compliance with the constitution -(all five principles satisfied). diff --git a/README.md b/README.md index 08673b8..5a97ca8 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ includes: ### `clock.disallowDateTimeNow` — PSR-20 Clock Abstraction -Enforces the [PSR-20](https://www.php-fig.org/psr/psr-20/) recommendation to avoid instantiating `DateTime` or `DateTimeImmutable` with the current time directly. This makes code that depends on the current time testable and respects the clock abstraction. +Enforces the [PSR-20](https://www.php-fig.org/psr/psr-20/) recommendation to avoid instantiating `DateTime` or `DateTimeImmutable` with a relative or implicit date. This makes code that depends on the current time testable and respects the clock abstraction. **Triggers on:** @@ -40,13 +40,18 @@ $a = new DateTime(); $b = new DateTime('now'); $c = new DateTimeImmutable(); $d = new DateTimeImmutable('now'); +$e = new DateTimeImmutable('yesterday'); +$f = new DateTimeImmutable('tomorrow'); +$g = new DateTime('+1 day'); +$h = new DateTimeImmutable('next Monday'); ``` -**Does not trigger on** (explicit non-"now" timestamps are fine): +**Does not trigger on** (absolute date strings and variables are allowed): ```php $a = new DateTime('2023-01-01'); -$b = new DateTimeImmutable('yesterday'); +$b = new DateTimeImmutable('2023-12-31 23:59:59'); +$c = new DateTimeImmutable($dateVariable); ``` **Recommended fix:** inject `Psr\Clock\ClockInterface` and call `$clock->now()`: diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 8d214b0..bf25017 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1,5 +1,11 @@ parameters: ignoreErrors: + - + message: '#^Avoid using new DateTime\(''\+1 day''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + - message: '#^Avoid using new DateTime\(''now''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' identifier: clock.disallowDateTimeNow @@ -12,12 +18,120 @@ parameters: count: 1 path: tests/Rules/Psr/fixtures/psr20.php + - + message: '#^Avoid using new DateTimeImmutable\(''\+0 seconds''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''\-2 weeks''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''0 days''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''2 days ago''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''TOMORROW''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''friday''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''in 3 weeks''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''last day of this month''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''mon''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''monday 14\:00\:00''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''next Monday''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''noon''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + - message: '#^Avoid using new DateTimeImmutable\(''now''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' identifier: clock.disallowDateTimeNow count: 1 path: tests/Rules/Psr/fixtures/psr20.php + - + message: '#^Avoid using new DateTimeImmutable\(''today midnight''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''today''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''tomorrow 12\:00''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''tomorrow''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''yesterday noon''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + + - + message: '#^Avoid using new DateTimeImmutable\(''yesterday''\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowDateTimeNow + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php + - message: '#^Avoid using new DateTimeImmutable\(\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' identifier: clock.disallowDateTimeNow @@ -25,10 +139,10 @@ parameters: path: tests/Rules/Psr/fixtures/psr20.php - - message: '#^Avoid using time\(\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' - identifier: clock.disallowTimeFunctions - count: 2 - path: tests/Rules/Psr/fixtures/psr20_functions.php + message: '#^Instantiating DateTimeImmutable with in 3 weeks produces an error\: The timezone could not be found in the database$#' + identifier: new.dateTimeImmutable + count: 1 + path: tests/Rules/Psr/fixtures/psr20.php - message: '#^Avoid using date\(\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' @@ -36,6 +150,12 @@ parameters: count: 4 path: tests/Rules/Psr/fixtures/psr20_functions.php + - + message: '#^Avoid using time\(\) directly\. Prefer using \\Psr\\Clock\\ClockInterface instead\.$#' + identifier: clock.disallowTimeFunctions + count: 2 + path: tests/Rules/Psr/fixtures/psr20_functions.php + - message: '#^Parameter \#2 \$timestamp of function date expects int\|null, string given\.$#' identifier: argument.type diff --git a/specs/002-psr20-ban-relative-dates/checklists/requirements.md b/specs/002-psr20-ban-relative-dates/checklists/requirements.md new file mode 100644 index 0000000..de2ec5b --- /dev/null +++ b/specs/002-psr20-ban-relative-dates/checklists/requirements.md @@ -0,0 +1,36 @@ +# Specification Quality Checklist: PSR-20 Relative Date Enforcement + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-07-22 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- All items pass. Specification is ready for `/speckit-plan`. +- The error identifier update (existing `clock.disallowDateTimeNow`) is explicitly deferred to planning as an assumption, keeping the spec free of implementation detail. +- PHP is referenced as the subject domain (not an implementation choice) since this is a PHP static analysis tool — this is acceptable for a developer-tool spec. diff --git a/specs/002-psr20-ban-relative-dates/contracts/rule-behavior.md b/specs/002-psr20-ban-relative-dates/contracts/rule-behavior.md new file mode 100644 index 0000000..f0c96f0 --- /dev/null +++ b/specs/002-psr20-ban-relative-dates/contracts/rule-behavior.md @@ -0,0 +1,74 @@ +# Contract: Psr20Rule Behavioral Contract + +**Rule identifier**: `clock.disallowDateTimeNow` +**Applies to**: `new DateTime(...)` and `new DateTimeImmutable(...)` constructor expressions. + +--- + +## Input + +A PHP AST node representing `new DateTime` or `new DateTimeImmutable` with zero or more arguments. + +--- + +## Output Contract + +### Violations (error reported) + +The rule MUST report an error for each of the following cases: + +| Case | Example | Error message | +|------|---------|---------------| +| No argument | `new DateTimeImmutable()` | `Avoid using new DateTimeImmutable() directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| String literal `now` (any case) | `new DateTimeImmutable('now')` | `Avoid using new DateTimeImmutable('now') directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| Relative date keyword | `new DateTimeImmutable('tomorrow')` | `Avoid using new DateTimeImmutable('tomorrow') directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| Relative day name | `new DateTimeImmutable('monday')` | `Avoid using new DateTimeImmutable('monday') directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| Signed offset | `new DateTimeImmutable('+1 day')` | `Avoid using new DateTimeImmutable('+1 day') directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| Unsigned offset | `new DateTimeImmutable('1 day')` | `Avoid using new DateTimeImmutable('1 day') directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| Ago expression | `new DateTimeImmutable('2 days ago')` | `Avoid using new DateTimeImmutable('2 days ago') directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| In expression | `new DateTimeImmutable('in 3 weeks')` | `Avoid using new DateTimeImmutable('in 3 weeks') directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| Next/last qualifier | `new DateTimeImmutable('next Monday')` | `Avoid using new DateTimeImmutable('next Monday') directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| Day-of-month selector | `new DateTimeImmutable('last day of this month')` | `Avoid using new DateTimeImmutable('last day of this month') directly. Prefer using \Psr\Clock\ClockInterface instead.` | +| Uppercase relative string | `new DateTimeImmutable('TOMORROW')` | `Avoid using new DateTimeImmutable('TOMORROW') directly. Prefer using \Psr\Clock\ClockInterface instead.` | + +### No Violation (passes silently) + +The rule MUST NOT report an error for: + +| Case | Example | +|------|---------| +| Variable argument | `new DateTimeImmutable($dateString)` | +| ISO date literal | `new DateTimeImmutable('2023-01-15')` | +| ISO date-time literal | `new DateTimeImmutable('2023-12-31 23:59:59')` | +| ISO 8601 with timezone | `new DateTimeImmutable('2023-01-15T12:00:00+00:00')` | +| Time-only literal | `new DateTimeImmutable('12:00:00')` | +| Variadic placeholder | `new DateTimeImmutable(...$args)` | +| Non-`DateTime`/`DateTimeImmutable` class | `new Carbon('tomorrow')` | + +--- + +## Error Message Template + +``` +Avoid using new {ClassName}({QuotedArgument}) directly. Prefer using \Psr\Clock\ClockInterface instead. +``` + +Where: +- `{ClassName}` is `DateTime` or `DateTimeImmutable` +- `{QuotedArgument}` is the quoted string literal (e.g., `'tomorrow'`) or empty string for no-argument case + +--- + +## Unchanged Behaviors (regression surface) + +The following behaviors from the pre-existing rule MUST be preserved: + +| Case | Expected outcome | +|------|-----------------| +| `new DateTime()` | Error: `Avoid using new DateTime() directly...` | +| `new DateTime('now')` | Error: `Avoid using new DateTime('now') directly...` | +| `new DateTimeImmutable()` | Error: `Avoid using new DateTimeImmutable() directly...` | +| `new DateTimeImmutable('now')` | Error: `Avoid using new DateTimeImmutable('now') directly...` | +| `new DateTime('2023-01-01')` | No error | +| `new DateTime($variable)` | No error | +| `new SomeOtherClass('tomorrow')` | No error | diff --git a/specs/002-psr20-ban-relative-dates/data-model.md b/specs/002-psr20-ban-relative-dates/data-model.md new file mode 100644 index 0000000..e08f115 --- /dev/null +++ b/specs/002-psr20-ban-relative-dates/data-model.md @@ -0,0 +1,95 @@ +# Data Model: PSR-20 Relative Date Enforcement + +This feature introduces no persistent data. The key design entity is the **relative date pattern set** — a static, in-code catalogue of patterns the rule tests against. + +--- + +## Entity: RelativeDatePattern + +**What it represents**: A single rule that classifies a PHP datetime string literal as "relative" (i.e., clock-dependent at runtime). + +| Attribute | Type | Description | +|-----------|------|-------------| +| `group` | string | Human-readable category (e.g., `keyword`, `signed-offset`) | +| `pattern` | regex | Case-insensitive PCRE pattern applied to the trimmed literal value | +| `examples` | string[] | Representative string literals that this pattern matches | + +**Relationships**: The full set of `RelativeDatePattern` instances together forms the detection catalogue used by `Psr20Rule::isRelativeDateString()`. + +--- + +## Catalogue: Relative Date Patterns + +The complete set of patterns, grouped for clarity: + +### Keywords + +| Pattern | Matches | +|---------|---------| +| `/^(now\|yesterday\|today\|tomorrow\|noon\|midnight)\b/i` | `now`, `tomorrow`, `noon`, `TOMORROW`, `yesterday noon`, `today midnight` | + +> **Why `\b` instead of `$`**: Using a word boundary rather than end-of-string allows the pattern +> to also catch compound modifier strings such as `'yesterday noon'` or `'tomorrow 12:00'`, where +> a relative keyword is followed by a time component. PHP's datetime parser treats these as +> relative expressions. The six keywords are consolidated into one pattern for efficiency. + +### Standalone Day Names + +| Pattern | Matches | +|---------|---------| +| `/^(monday\|tuesday\|wednesday\|thursday\|friday\|saturday\|sunday)\b/i` | `Monday`, `friday`, `TUESDAY`, `monday 14:00:00` | + +> **Why `\b` instead of `$`**: Same rationale as Keywords — day names followed by a time +> component (e.g., `'monday 14:00:00'`) are relative and must be caught. + +### Relative Qualifiers + +| Pattern | Matches | +|---------|---------| +| `/^(next\|last\|previous\|this)\s+/i` | `next Monday`, `last week`, `previous year`, `this month` | + +### Offset Formats + +| Pattern | Matches | +|---------|---------| +| `/^[+-]\d+\s+(second\|minute\|hour\|day\|week\|month\|year)s?/i` | `+1 day`, `-2 weeks`, `+3 months`, `-1 second` | +| `/^\d+\s+(second\|minute\|hour\|day\|week\|month\|year)s?\s*$/i` | `1 day`, `2 weeks`, `3 months` | + +### Ago / In + +| Pattern | Matches | +|---------|---------| +| `/\bago\b/i` | `2 days ago`, `1 week ago`, `3 months ago` | +| `/^in\s+\d+\s+(second\|minute\|hour\|day\|week\|month\|year)s?/i` | `in 2 days`, `in 1 week`, `in 3 months` | + +### Day-of-Month Selectors + +| Pattern | Matches | +|---------|---------| +| `/^(first\|last)\s+day\s+of\b/i` | `first day of this month`, `last day of January 2023` | + +### Clock-Relative + +| Pattern | Matches | +|---------|---------| +| `/^(back\|front)\s+of\b/i` | `back of 10`, `front of 11` | + +--- + +## Non-Entity: AbsoluteDateString + +An absolute date string is defined negatively — any string literal that matches none of the patterns above is treated as absolute and allowed. Common absolute formats that the rule correctly allows: + +| Format | Example | +|--------|---------| +| ISO 8601 date | `2023-01-15` | +| ISO 8601 date-time | `2023-01-15 12:00:00` | +| ISO 8601 with timezone | `2023-01-15T12:00:00+00:00` | +| Time only | `12:00:00` | +| US format | `01/15/2023` | + +--- + +## State Transitions + +None — the rule is stateless. Each constructor invocation is evaluated independently. diff --git a/specs/002-psr20-ban-relative-dates/plan.md b/specs/002-psr20-ban-relative-dates/plan.md new file mode 100644 index 0000000..08657a4 --- /dev/null +++ b/specs/002-psr20-ban-relative-dates/plan.md @@ -0,0 +1,125 @@ +# Implementation Plan: PSR-20 Relative Date Enforcement + +**Branch**: `002-psr20-ban-relative-dates` | **Date**: 2026-07-22 | **Spec**: [spec.md](spec.md) + +**Input**: Feature specification from `specs/002-psr20-ban-relative-dates/spec.md` + +## Summary + +Extend `Psr20Rule` to detect all PHP relative-date string literals passed to `new DateTime` or `new DateTimeImmutable` constructors, not just `'now'` and the empty case. The rule adds a private static method that tests string literals against a compiled catalogue of PCRE patterns covering PHP's documented relative date formats. Absolute date strings and variable arguments continue to pass. The rule identifier `clock.disallowDateTimeNow` and error message format are preserved for backward compatibility. Tests, fixture, and README documentation are updated accordingly. + +## Technical Context + +**Language/Version**: PHP ^8.2 + +**Primary Dependencies**: PHPStan ^2.0, php-parser (transitive via PHPStan), PHPUnit ^11 (dev) + +**Storage**: N/A + +**Testing**: PHPUnit 11 via `PHPStan\Testing\RuleTestCase` + +**Target Platform**: PHP CLI (static analysis tool — runs during development/CI) + +**Project Type**: PHPStan extension library + +**Performance Goals**: Rule evaluation per node is O(number of patterns) string matching — negligible overhead. + +**Constraints**: Must remain compatible with PHP 8.2–8.5 and PHPStan ^2.0. No new `require` dependencies. + +**Scale/Scope**: Single rule class, single test class, single fixture file, one README section. + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +| Principle | Status | Notes | +|-----------|--------|-------| +| P1 — Broad Compatibility | PASS | Changes use only PHP 8.2-compatible syntax; no new dependencies | +| P2 — Test Coverage per Rule | PASS | Existing `Psr20RuleTest` is updated; fixture extended with positive and negative cases | +| P3 — Documentation per Rule | PASS | README `clock.disallowDateTimeNow` section updated as part of this change | +| P4 — Open-Source Quality Standards | PASS | `final readonly` class retained; `strict_types=1` present; cs-fixer and phpstan must pass | +| P5 — Single-Responsibility Rule Design | PASS | Single `getNodeType()` return (`New_::class`); logic remains in one class | + +No violations. Complexity Tracking section not required. + +## Project Structure + +### Documentation (this feature) + +```text +specs/002-psr20-ban-relative-dates/ +├── plan.md # This file +├── research.md # Phase 0 — detection approach and decisions +├── data-model.md # Phase 1 — relative date pattern catalogue +├── quickstart.md # Phase 1 — end-to-end validation guide +├── contracts/ +│ └── rule-behavior.md # Phase 1 — triggering/non-triggering contract +├── checklists/ +│ └── requirements.md # Spec quality checklist +└── tasks.md # Phase 2 output (created by /speckit-tasks) +``` + +### Source Code (repository root) + +```text +src/ +└── Rules/ + └── Psr/ + └── Psr20Rule.php # Add isRelativeDateString(); update processNode() + +tests/ +└── Rules/ + └── Psr/ + ├── Psr20RuleTest.php # Update expected errors; add relative-date assertions + └── fixtures/ + └── psr20.php # Extend with relative and absolute date examples + +README.md # Update "Triggers on" / "Does not trigger on" for clock rule +``` + +**Structure Decision**: Single-project layout (Option 1). Changes are confined to the existing `src/Rules/Psr/` and `tests/Rules/Psr/` directories plus the README. + +## Design + +### Core Change: `Psr20Rule::isRelativeDateString()` + +A new private static method is added to `Psr20Rule`. It applies a catalogue of PCRE patterns (see [data-model.md](data-model.md)) against the trimmed string literal value (case-insensitive). Returns `true` if any pattern matches. + +`processNode()` is updated so that, after confirming the first argument is a `Node\Scalar\String_`, it calls `isRelativeDateString()` on the value. If `true`, the error is reported (replacing the existing `'now'`-only check). + +The `'now'` case is subsumed by the new keyword pattern `/^now$/i` and no longer requires special-case handling; the existing behavior is preserved. + +### Fixture and Test Updates + +The existing fixture (`tests/Rules/Psr/fixtures/psr20.php`) is extended with: + +**New error-triggering lines** (relative dates): +- `new DateTimeImmutable('yesterday')` +- `new DateTimeImmutable('tomorrow')` +- `new DateTimeImmutable('+1 day')` +- `new DateTimeImmutable('-2 weeks')` +- `new DateTimeImmutable('next Monday')` +- `new DateTimeImmutable('2 days ago')` +- `new DateTimeImmutable('last day of this month')` + +**Lines that must not trigger** (absolute dates): +- `new DateTimeImmutable('2023-01-15')` — already present; remains a non-error +- `new DateTimeImmutable('2023-12-31 23:59:59')` — add +- `new DateTimeImmutable('2023-01-15T12:00:00+00:00')` — add + +The test's expected-error array is updated to include all new triggering lines with their correct line numbers and error messages. + +### README Update + +The `clock.disallowDateTimeNow` rule section in `README.md` is updated: + +- **Triggers on** block: add representative relative-date examples (`'yesterday'`, `'+1 day'`, `'next Monday'`). +- **Does not trigger on** block: clarify that only absolute date strings and variable arguments pass; remove `'yesterday'` from the passing examples (it currently appears there incorrectly). + +### No New Dependencies + +The detection logic uses PHP's built-in `preg_match` only. No composer packages are added. + +## Complexity Tracking + +No constitution violations — table not required. diff --git a/specs/002-psr20-ban-relative-dates/quickstart.md b/specs/002-psr20-ban-relative-dates/quickstart.md new file mode 100644 index 0000000..dbe18d6 --- /dev/null +++ b/specs/002-psr20-ban-relative-dates/quickstart.md @@ -0,0 +1,105 @@ +# Quickstart Validation Guide: PSR-20 Relative Date Enforcement + +This guide describes how to validate the updated `Psr20Rule` end-to-end after implementation. + +All commands must be run inside the PHP container: + +```bash +docker compose run --rm php +``` + +--- + +## Prerequisites + +- Docker and Docker Compose installed and running +- Project dependencies installed: + +```bash +docker compose run --rm php composer install +``` + +--- + +## Validation Scenarios + +### Scenario 1: Relative date strings are flagged + +**Setup**: The updated test fixture (`tests/Rules/Psr/fixtures/psr20.php`) must include PHP code with relative date string literals such as `'tomorrow'`, `'+1 day'`, `'yesterday'`, `'next Monday'`, and compound modifiers like `'yesterday noon'` and `'monday 14:00:00'`. + +**Run**: + +```bash +docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php +``` + +**Expected outcome**: The test passes, confirming that each relative date literal in the fixture maps to an expected error at the correct line. + +--- + +### Scenario 2: Absolute date strings are not flagged + +**Setup**: The fixture must also include lines using absolute date strings such as `'2023-01-15'`, `'2023-12-31 23:59:59'`, and ISO 8601 strings. These lines must NOT appear in the expected-errors array in the test. + +**Run**: + +```bash +docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php +``` + +**Expected outcome**: Test passes with no unexpected errors for absolute date lines. + +--- + +### Scenario 3: Variable arguments are not flagged + +**Setup**: The fixture must include lines using variables as constructor arguments (e.g., `new DateTimeImmutable($someVar)`). These must NOT appear in the expected-errors array. + +**Run**: + +```bash +docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php +``` + +**Expected outcome**: Test passes with no errors for variable-argument lines. + +--- + +### Scenario 4: Existing behaviors are preserved + +**Setup**: The fixture retains the original lines (`new DateTime()`, `new DateTime('now')`, `new DateTimeImmutable()`, `new DateTimeImmutable('now')`). Expected errors for these lines must still be present in the test. + +**Run**: + +```bash +docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php +``` + +**Expected outcome**: All four existing error assertions still pass. + +--- + +### Scenario 5: Full CI pipeline passes + +**Run all three CI checks**: + +```bash +docker compose run --rm php composer test +docker compose run --rm php composer phpstan +docker compose run --rm php composer cs-check +``` + +**Expected outcome**: All three commands exit with code 0 on all supported PHP versions (8.2, 8.3, 8.4, 8.5). + +--- + +## Key Files Touched + +| File | Purpose | +|------|---------| +| `src/Rules/Psr/Psr20Rule.php` | Core rule logic — relative-date detection added | +| `tests/Rules/Psr/Psr20RuleTest.php` | Test — expected errors updated to include relative-date cases | +| `tests/Rules/Psr/fixtures/psr20.php` | Fixture — new lines for relative date and absolute date cases added | +| `README.md` | Rule documentation — "Triggers on" and "Does not trigger on" sections updated | + +See [rule behavior contract](contracts/rule-behavior.md) for the complete list of triggering and non-triggering cases. diff --git a/specs/002-psr20-ban-relative-dates/research.md b/specs/002-psr20-ban-relative-dates/research.md new file mode 100644 index 0000000..af34edf --- /dev/null +++ b/specs/002-psr20-ban-relative-dates/research.md @@ -0,0 +1,80 @@ +# Research: PSR-20 Relative Date Enforcement + +## Decision 1: How to detect "relative" vs "absolute" date strings + +**Decision**: Use a compiled allowlist of regex patterns matching PHP's documented relative date formats. Flag a string literal if it matches any pattern; allow it otherwise. + +**Rationale**: PHP's datetime parser accepts a large set of relative specifiers (documented at https://www.php.net/manual/en/datetime.formats.relative.php). Rather than trying to parse what is "absolute" (very wide set), it is simpler and more predictable to enumerate what is "relative" and flag exactly those patterns. False negatives (missing an obscure relative string) are acceptable; false positives (blocking a legitimate absolute date) are not. + +**Alternatives considered**: +- **Call `DateTimeImmutable::createFromFormat`** at rule-evaluation time to test if the string matches a known absolute format: rejected because it introduces runtime evaluation inside a static analysis rule and requires a known list of absolute formats that may be incomplete. +- **Attempt full parsing with PHP's date parser**: rejected because the parser is highly permissive and many absolute strings would still parse as "relative" in certain contexts. + +--- + +## Decision 2: Relative date patterns to detect + +The following pattern groups cover the PHP-documented relative date formats: + +| Group | Examples | Pattern strategy | +|-------|----------|-----------------| +| Keywords | `now`, `yesterday`, `today`, `tomorrow` | Exact keyword match (case-insensitive) | +| Day of day | `noon`, `midnight` | Exact keyword match | +| Day names (standalone) | `monday`, `friday` | Exact weekday name match | +| Relative qualifiers | `next monday`, `last friday`, `previous year`, `this week` | `^(next\|last\|previous\|this)\s+` prefix | +| Signed offsets | `+1 day`, `-2 weeks`, `+3 months` | `^[+-]\d+\s+` | +| Unsigned offsets with unit | `1 day`, `2 weeks` (when followed by a known time unit) | `^\d+\s+(second\|minute\|hour\|day\|week\|month\|year)s?` | +| Ago | `2 days ago`, `1 week ago` | `\bago\b` anywhere in string | +| In N units | `in 2 days`, `in 1 week` | `^in\s+\d+` prefix | +| Day-of-month selectors | `first day of`, `last day of` | `^(first\|last)\s+day\s+of\b` | +| Clock-relative | `back of 10`, `front of 11` | `^(back\|front)\s+of\b` | + +All matching is case-insensitive. Matching is applied to the trimmed string value. + +**Edge case: `today`** — PHP treats `today` as midnight of the current day, making it relative. The rule flags it. + +**Edge case: time-only strings** (e.g., `'12:00:00'`) — These are absolute time references with no date component. They match none of the relative patterns and are correctly allowed. + +**Edge case: `'+0 seconds'`** — Matches the signed-offset pattern; flagged correctly. + +**Edge case: `'last day of January 2023'`** — Contains `last day of` prefix; flagged (has a relative component). + +--- + +## Decision 3: Rule identifier — keep or replace `clock.disallowDateTimeNow` + +**Decision**: Keep the existing `clock.disallowDateTimeNow` identifier. + +**Rationale**: Any user who has suppressed this error with `// @phpstan-ignore clock.disallowDateTimeNow` would face a breaking change if the identifier changed. The identifier's semantic scope is "PSR-20 clock abstraction compliance", not narrowly "only the `now` literal". Keeping it backward-compatible is the right trade-off. + +**Alternatives considered**: +- **New identifier `clock.disallowRelativeDateTime`**: provides clearer semantics but breaks existing suppressions without a migration path. Rejected in favor of backward compatibility. +- **Emit both old and new identifiers**: PHPStan `RuleErrorBuilder` supports a single identifier per error. Not feasible without a refactor. Rejected. + +--- + +## Decision 4: Error message format + +**Decision**: Extend the existing error message format to name the offending literal. The current format `"Avoid using new $class($context) directly. Prefer using \Psr\Clock\ClockInterface instead."` already supports variable `$context`, so `'tomorrow'`, `'+1 day'`, etc. are naturally substituted in. No structural change needed. + +**Rationale**: The message naturally extends without modification. For empty args, `$context` remains `''` (current behavior). For string literals, `$context` is the quoted string value (e.g., `'tomorrow'`). Consistent with the existing pattern. + +--- + +## Decision 5: Handling the `DateTime` class alongside `DateTimeImmutable` + +**Decision**: Apply the same relative-date detection to both `DateTime` and `DateTimeImmutable`, consistent with the existing rule behavior. + +**Rationale**: Both classes share the same constructor signature and accept the same date formats. The PSR-20 concern (clock dependency) applies equally to both. + +--- + +## Summary of Unknowns Resolved + +| Unknown | Resolution | +|---------|-----------| +| How to classify relative vs absolute strings | Regex allowlist of relative patterns | +| Which relative patterns to cover | PHP documented relative formats (10 pattern groups) | +| Rule identifier strategy | Keep `clock.disallowDateTimeNow` for backward compat | +| Error message changes | None required — existing format already handles it | +| Scope: both `DateTime` and `DateTimeImmutable` | Yes, consistent with existing behavior | diff --git a/specs/002-psr20-ban-relative-dates/spec.md b/specs/002-psr20-ban-relative-dates/spec.md new file mode 100644 index 0000000..3d0d8ef --- /dev/null +++ b/specs/002-psr20-ban-relative-dates/spec.md @@ -0,0 +1,109 @@ +# Feature Specification: PSR-20 Relative Date Enforcement + +**Feature Branch**: `002-psr20-ban-relative-dates` + +**Created**: 2026-07-22 + +**Status**: Draft + +**Input**: User description: "@src/Rules/Psr/Psr20Rule.php currently act only against current timestamp. It needs to be updated to forbid any relative date (tomorrow, +1 day, etc...) but keep possible to instanciate a DateTimeImmutable via its constructor if the date is a variable or an absolute date represented as a string." + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Relative Date Strings Are Flagged (Priority: P1) + +A developer writes code that constructs a `DateTime` or `DateTimeImmutable` object using a relative date string such as `'tomorrow'`, `'+1 day'`, `'next Monday'`, or `'last week'`. The rule detects this as a violation and directs the developer to use a clock abstraction instead. + +**Why this priority**: This is the core new capability. The existing rule only blocks `'now'` and empty constructors, leaving many implicit time-dependency patterns undetected. Catching all relative strings is the fundamental goal of this feature. + +**Independent Test**: Can be fully tested by running static analysis on a file containing `new DateTimeImmutable('tomorrow')` and verifying that a violation is reported. + +**Acceptance Scenarios**: + +1. **Given** code containing `new DateTimeImmutable('tomorrow')`, **When** static analysis runs, **Then** a violation is reported directing the developer to use a clock interface. +2. **Given** code containing `new DateTime('+1 day')`, **When** static analysis runs, **Then** a violation is reported. +3. **Given** code containing `new DateTimeImmutable('next Monday')`, **When** static analysis runs, **Then** a violation is reported. +4. **Given** code containing `new DateTimeImmutable('-2 weeks')`, **When** static analysis runs, **Then** a violation is reported. +5. **Given** code containing `new DateTimeImmutable('yesterday')`, **When** static analysis runs, **Then** a violation is reported. +6. **Given** code containing `new DateTimeImmutable('TOMORROW')` (uppercase), **When** static analysis runs, **Then** a violation is reported. +7. **Given** code containing `new DateTimeImmutable('last day of this month')`, **When** static analysis runs, **Then** a violation is reported. + +--- + +### User Story 2 - Absolute Date Strings Are Permitted (Priority: P2) + +A developer constructs a `DateTime` or `DateTimeImmutable` object using a hard-coded absolute date string (e.g., `'2023-01-15'` or `'2023-01-15 12:00:00'`). This is a valid pattern — for example, when setting a known fixed date in a test or configuration — and the rule must not report a violation. + +**Why this priority**: Without this allowance the rule would block legitimate uses of explicitly known dates, making it unnecessarily obstructive. Distinguishing relative from absolute is what makes the rule precise and trustworthy. + +**Independent Test**: Can be fully tested by running static analysis on a file containing `new DateTimeImmutable('2023-01-15')` and verifying no violation is reported. + +**Acceptance Scenarios**: + +1. **Given** code containing `new DateTimeImmutable('2023-01-15')`, **When** static analysis runs, **Then** no violation is reported. +2. **Given** code containing `new DateTime('2023-12-31 23:59:59')`, **When** static analysis runs, **Then** no violation is reported. +3. **Given** code containing `new DateTimeImmutable('2023-01-15T12:00:00+00:00')`, **When** static analysis runs, **Then** no violation is reported. + +--- + +### User Story 3 - Variable Arguments Are Permitted (Priority: P2) + +A developer constructs a `DateTime` or `DateTimeImmutable` object using a variable as the date argument. Because the value is not known at analysis time, the rule allows this usage to avoid false positives. + +**Why this priority**: Blocking variable-based construction would produce false positives in legitimate dynamic scenarios, undermining developer trust in the rule. + +**Independent Test**: Can be fully tested by running static analysis on a file containing `new DateTimeImmutable($dateString)` and verifying no violation is reported. + +**Acceptance Scenarios**: + +1. **Given** code containing `new DateTimeImmutable($dateString)`, **When** static analysis runs, **Then** no violation is reported. +2. **Given** code containing `new DateTime($someVariable)`, **When** static analysis runs, **Then** no violation is reported. + +--- + +### Edge Cases + +- What happens with `'now'` (existing behavior)? → Must still be flagged. +- What happens with an empty constructor `new DateTimeImmutable()` (existing behavior)? → Must still be flagged. +- What happens with a variadic placeholder argument (`...`)? → Must not be flagged (cannot be statically evaluated). +- What happens with relative strings in mixed case (e.g., `'TOMORROW'`, `'Next Monday'`)? → Must be flagged (case-insensitive matching). +- What happens with `'+0 seconds'` or `'0 days'`? → Must be flagged (still a relative expression, even if the offset is zero). +- What happens with time-only strings like `'12:00:00'`? → Must not be flagged (represents an absolute time, not a relative date). +- What happens with strings that combine a fixed month/year and a relative selector (e.g., `'last day of January 2023'`)? → Must be flagged (contains a relative component). + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: The rule MUST report a violation when `new DateTime` or `new DateTimeImmutable` is called with no arguments. +- **FR-002**: The rule MUST report a violation when `new DateTime` or `new DateTimeImmutable` is called with the string literal `'now'` (case-insensitive). +- **FR-003**: The rule MUST report a violation when `new DateTime` or `new DateTimeImmutable` is called with a string literal that represents a relative date or time. +- **FR-004**: Relative date string detection MUST cover at minimum: `yesterday`, `today`, `tomorrow`, `next `, `last `, `+N `, `-N `, `first day of`, `last day of`, `ago`, and other English-language relative specifiers recognized by the PHP datetime parser. +- **FR-005**: Relative date matching MUST be case-insensitive. +- **FR-006**: The rule MUST NOT report a violation when the first constructor argument is a variable (value not known at analysis time). +- **FR-007**: The rule MUST NOT report a violation when the first constructor argument is a string literal representing an absolute date or date-time (e.g., `'2023-01-15'`, `'2023-12-31 23:59:59'`, ISO 8601 strings). +- **FR-008**: The rule MUST NOT report a violation when the first constructor argument is a variadic placeholder. +- **FR-009**: The error message MUST direct the developer to use `\Psr\Clock\ClockInterface` instead. + +### Key Entities + +- **Relative date string**: A string literal whose value is computed relative to the current moment at runtime (e.g., `'tomorrow'`, `'+1 day'`, `'next Monday'`, `'last week'`, `'yesterday noon'`). +- **Absolute date string**: A string literal that denotes a fixed, explicitly stated date or date-time whose value does not depend on when the code executes (e.g., `'2023-01-15'`, `'2023-01-15 12:00:00'`). + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: 100% of relative date string patterns in the test fixture are detected as violations by the updated rule. +- **SC-002**: 0 false positives are produced when the rule analyzes code using absolute date string literals or variable arguments. +- **SC-003**: All previously existing passing tests for the rule continue to pass without modification. +- **SC-004**: The full CI pipeline (tests, static analysis, code style) passes on all declared supported PHP versions. + +## Assumptions + +- The rule targets `DateTime` and `DateTimeImmutable` by their unqualified names, consistent with the current behavior. Fully qualified names (e.g., `\DateTimeImmutable`) are also covered. +- The scope does not extend to subclasses, interfaces, or third-party date libraries. +- The definition of "relative date string" is derived from the set of relative formats recognized by the PHP datetime parser, not from a custom arbitrary list. +- Strings that are ambiguous but whose most natural reading is absolute (e.g., pure time strings like `'12:00:00'`) are treated as absolute to minimize false positives. +- The existing rule error identifier may need to be broadened in scope; this is deferred to the planning phase. +- No new dependencies are introduced to implement the detection logic. diff --git a/specs/002-psr20-ban-relative-dates/tasks.md b/specs/002-psr20-ban-relative-dates/tasks.md new file mode 100644 index 0000000..817256f --- /dev/null +++ b/specs/002-psr20-ban-relative-dates/tasks.md @@ -0,0 +1,183 @@ +# Tasks: PSR-20 Relative Date Enforcement + +**Input**: Design documents from `specs/002-psr20-ban-relative-dates/` + +**Prerequisites**: plan.md ✓, spec.md ✓, research.md ✓, data-model.md ✓, contracts/ ✓ + +**Organization**: Tasks are grouped by user story to enable independent implementation and testing of each story. + +## Format: `[ID] [P?] [Story] Description` + +- **[P]**: Can run in parallel (different files, no dependencies) +- **[Story]**: Which user story this task belongs to (US1, US2, US3) + +--- + +## Phase 1: Setup + +**Purpose**: Verify development environment is ready. No project initialization needed — all structure exists. + +- [x] T001 Verify project dependencies are installed: `docker compose run --rm php composer install` + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: Add the relative-date detection method to the rule class. All user stories depend on this. + +**⚠️ CRITICAL**: No user story work can begin until this phase is complete. + +- [x] T002 Add private static method `isRelativeDateString(string $value): bool` to `src/Rules/Psr/Psr20Rule.php` — implement all 10 PCRE pattern groups from `specs/002-psr20-ban-relative-dates/data-model.md` (keywords, day names, relative qualifiers, signed/unsigned offsets, ago, in, day-of-month selectors, clock-relative); matching must be case-insensitive on the trimmed string value +- [x] T003 Update `processNode()` in `src/Rules/Psr/Psr20Rule.php` — replace the `'now' === strtolower($firstArg->value)` check with `self::isRelativeDateString($firstArg->value)` and pass the raw string value (not the quoted form) as `$context` to `buildError()` wrapped in single-quotes; the `'now'` case is now subsumed by the keyword pattern + +**Checkpoint**: Rule logic is updated. Run `docker compose run --rm php composer phpstan` — must pass. + +--- + +## Phase 3: User Story 1 — Relative date strings are flagged (Priority: P1) 🎯 MVP + +**Goal**: All PHP relative date string literals passed to `new DateTime`/`new DateTimeImmutable` are reported as violations. + +**Independent Test**: Run `docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php` — all expected errors for relative-date lines must be present. + +### Implementation for User Story 1 + +- [x] T004 [US1] Extend `tests/Rules/Psr/fixtures/psr20.php` — append lines using relative date string literals that must trigger errors: `new DateTimeImmutable('yesterday')` is already on line 10 (leave it); add `new DateTimeImmutable('tomorrow')`, `new DateTime('+1 day')`, `new DateTimeImmutable('next Monday')`, `new DateTimeImmutable('-2 weeks')`, `new DateTimeImmutable('2 days ago')`, `new DateTimeImmutable('last day of this month')`; also cover remaining pattern groups from data-model.md: `new DateTimeImmutable('today')` (keyword), `new DateTimeImmutable('friday')` (standalone day name), `new DateTimeImmutable('in 3 weeks')` (in-expression), `new DateTimeImmutable('noon')` (clock keyword); and edge cases: `new DateTimeImmutable('+0 seconds')` and `new DateTimeImmutable('0 days')` (zero-offset must still flag) +- [x] T005 [US1] Update `tests/Rules/Psr/Psr20RuleTest.php` — add expected error assertions for every relative-date line in the fixture, including the pre-existing `'yesterday'` line (currently line 10, now expected to error); each assertion must match the exact error message format `"Avoid using new {Class}('{arg}') directly. Prefer using \Psr\Clock\ClockInterface instead."` and the correct line number +- [x] T006 [US1] Run `docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php` and confirm all assertions pass + +**Checkpoint**: User Story 1 complete. Every relative date literal in the fixture produces the expected error. Existing errors for empty-arg and `'now'` cases still pass. + +--- + +## Phase 4: User Story 2 — Absolute date strings are permitted (Priority: P2) + +**Goal**: String literals representing absolute dates (e.g., `'2023-01-15'`) produce no violation. + +**Independent Test**: Run `docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php` — no errors for absolute-date lines. + +### Implementation for User Story 2 + +- [x] T007 [US2] Extend `tests/Rules/Psr/fixtures/psr20.php` — add absolute date lines that must NOT trigger errors: `new DateTime('2023-01-01')` is already present (leave it); add `new DateTimeImmutable('2023-12-31 23:59:59')`, `new DateTimeImmutable('2023-01-15T12:00:00+00:00')`, and `new DateTimeImmutable('12:00:00')` (time-only string — must not flag) +- [x] T008 [US2] Confirm those lines are absent from the expected-error array in `tests/Rules/Psr/Psr20RuleTest.php` (no assertion added for them) and run `docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php` + +**Checkpoint**: User Stories 1 and 2 both pass. Absolute date lines produce no errors; relative date lines all produce errors. + +--- + +## Phase 5: User Story 3 — Variable arguments are permitted (Priority: P2) + +**Goal**: Variable arguments (`$var`) passed to `new DateTime`/`new DateTimeImmutable` produce no violation. + +**Independent Test**: Run `docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php` — no errors for variable-arg lines. + +### Implementation for User Story 3 + +- [x] T009 [US3] Extend `tests/Rules/Psr/fixtures/psr20.php` — add variable argument lines that must NOT trigger errors: `new DateTimeImmutable($dateString)` and `new DateTime($someVar)` (declare `$dateString` and `$someVar` as `string` variables above them); also add `new DateTimeImmutable(...$args)` (variadic placeholder — covers FR-008; declare `$args` as `string[]` above it) +- [x] T010 [US3] Confirm those lines are absent from the expected-error array in `tests/Rules/Psr/Psr20RuleTest.php` and run `docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php` + +**Checkpoint**: All three user stories pass. Full test suite is green. + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +**Purpose**: Documentation update and full CI validation. + +- [x] T011 [P] Update `README.md` section for `clock.disallowDateTimeNow` — add `'tomorrow'`, `'+1 day'`, `'next Monday'` to the "Triggers on" code block; remove `'yesterday'` from the "Does not trigger on" block (it now triggers); add `new DateTimeImmutable($dateVariable)` as an allowed example in "Does not trigger on" +- [x] T012 Run full CI pipeline: `docker compose run --rm php composer test && docker compose run --rm php composer phpstan && docker compose run --rm php composer cs-check` — all three must exit 0 + +--- + +## Dependencies & Execution Order + +### Phase Dependencies + +- **Setup (Phase 1)**: No dependencies — can start immediately +- **Foundational (Phase 2)**: Depends on Phase 1 — **BLOCKS all user stories** +- **User Stories (Phases 3–5)**: All depend on Foundational (Phase 2) + - US1 (Phase 3) must complete first — US2 and US3 fixture tasks append to the same file + - US2 (Phase 4) depends on Phase 3 completing (fixture file must be stable) + - US3 (Phase 5) depends on Phase 4 completing (same fixture file) +- **Polish (Phase 6)**: Depends on all user stories complete + +### User Story Dependencies + +- **US1 (P1)**: Can start after Phase 2 — no dependency on US2/US3 +- **US2 (P2)**: Logically independent but shares the fixture file with US1 — sequence after US1 to avoid conflicts +- **US3 (P2)**: Logically independent but shares the fixture file — sequence after US2 + +### Within Each User Story + +- Fixture extension before test update (need line numbers to write assertions) +- Test update before test run +- Logic change (Phase 2) before any fixture/test work + +### Parallel Opportunities + +- T002 and T003 in Phase 2 must be sequential (T003 depends on T002's change) +- T004, T007, T009 are NOT marked [P] — they all modify the same fixture file and must be sequenced by phase +- T011 (README) is [P] relative to the test run (T012) — can be done while T012 runs + +--- + +## Parallel Example: Foundational Phase + +```bash +# Phase 2 must be sequential — T003 uses the method added in T002: +Task T002: "Add isRelativeDateString() to src/Rules/Psr/Psr20Rule.php" + → then → +Task T003: "Update processNode() in src/Rules/Psr/Psr20Rule.php" +``` + +## Parallel Example: Polish Phase + +```bash +# T011 and T012 can run in parallel: +Task T011: "Update README.md documentation" +Task T012: "Run full CI pipeline" +``` + +--- + +## Implementation Strategy + +### MVP First (User Story 1 Only) + +1. Complete Phase 1: Setup (verify env) +2. Complete Phase 2: Foundational (add detection logic — **required**) +3. Complete Phase 3: User Story 1 (relative dates flagged) +4. **STOP and VALIDATE**: Run `docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php` +5. Core capability delivered + +### Incremental Delivery + +1. Phase 1 + Phase 2 → Detection logic ready +2. Phase 3 (US1) → Relative dates flagged → MVP delivered +3. Phase 4 (US2) → Absolute dates confirmed passing → Extra test coverage +4. Phase 5 (US3) → Variable args confirmed passing → Full test coverage +5. Phase 6 → README updated + CI green → Ready to merge + +### Single-Developer Strategy + +Work strictly in phase order (1 → 2 → 3 → 4 → 5 → 6). No parallel work needed — this is a single-developer feature touching a small number of files. + +--- + +## Notes + +- [P] tasks = different files or logically independent, no file conflicts at that phase +- [Story] label maps each task to its spec user story for traceability +- The fixture file (`psr20.php`) is extended in multiple phases — always append, never rewrite earlier lines, to preserve line numbers from prior assertions +- Verify line numbers in the fixture match expected-error line numbers in the test before each run +- `'yesterday'` is on line 10 of the existing fixture and currently does NOT error — after T003 (Foundational), it will error; T005 must add this to the expected-error array +- Run `composer cs-check` after any PHP file change to catch formatting issues early + +--- + +## Phase 7: Convergence + +- [x] T013 [US1] Add `new DateTimeImmutable('TOMORROW')` to `tests/Rules/Psr/fixtures/psr20.php` (append after line 35) and add the corresponding expected error assertion in `tests/Rules/Psr/Psr20RuleTest.php`; then regenerate `phpstan-baseline.neon` and run `docker compose run --rm php vendor/bin/phpunit tests/Rules/Psr/Psr20RuleTest.php` — verifies US1/AC6 (uppercase relative string is flagged) (partial) +- [x] T014 [P] Update `specs/002-psr20-ban-relative-dates/data-model.md` — replace the individual `$`-anchored per-keyword patterns (e.g., `/^now$/i`, `/^tomorrow$/i`) and the standalone day-name pattern with their consolidated `\b` equivalents that match the implementation; add a "Compound Modifiers" note explaining why `\b` is used instead of `$` (partial) +- [x] T015 [P] Add a non-`DateTime`/`DateTimeImmutable` class constructor line to `tests/Rules/Psr/fixtures/psr20.php` (e.g., `$cc = new \stdClass();`) and confirm no expected error for it in `tests/Rules/Psr/Psr20RuleTest.php`; verifies the contract's "Non-DateTime/DateTimeImmutable class MUST NOT trigger" case (partial) +- [x] T016 [P] Update `specs/002-psr20-ban-relative-dates/quickstart.md` Scenario 1 — add at least one compound modifier example (`'yesterday noon'` or `'monday 14:00:00'`) to the example list so the guide reflects the full implemented behavior (partial) diff --git a/src/Rules/Psr/Psr20Rule.php b/src/Rules/Psr/Psr20Rule.php index d79f5cb..c778573 100644 --- a/src/Rules/Psr/Psr20Rule.php +++ b/src/Rules/Psr/Psr20Rule.php @@ -15,7 +15,8 @@ /** * Enforce the PSR-20 recommendation to avoid using `new \DateTime()` or `new \DateTimeImmutable()` directly in your code. * - * It tracks either `new \DateTime()`, `new \DateTime('now')`, `new \DateTimeImmutable()` and `new \DateTimeImmutable('now')`. + * Flags empty constructors and any relative date string literal (e.g. `'now'`, `'tomorrow'`, `'+1 day'`, `'next Monday'`). + * Absolute date strings (e.g. `'2023-01-15'`) and variable arguments are allowed. * * @implements Rule */ @@ -50,13 +51,39 @@ public function processNode(Node $node, Scope $scope): array $firstArg = $firstArg->value; - if ($firstArg instanceof Node\Scalar\String_ && 'now' === strtolower($firstArg->value)) { - return [$this->buildError($node->class->toString(), '\'now\'')]; + if ($firstArg instanceof Node\Scalar\String_ && self::isRelativeDateString($firstArg->value)) { + return [$this->buildError($node->class->toString(), sprintf("'%s'", $firstArg->value))]; } return []; } + private static function isRelativeDateString(string $value): bool + { + $trimmed = trim($value); + + $patterns = [ + '/^(now|yesterday|today|tomorrow|noon|midnight)\b/i', + '/^(monday|tuesday|wednesday|thursday|friday|saturday|sunday)\b/i', + '/^(mon|tue|wed|thu|fri|sat|sun)\b/i', + '/^(next|last|previous|this)\s+/i', + '/^[+-]\d+\s+(second|minute|hour|day|week|month|year)s?/i', + '/^\d+\s+(second|minute|hour|day|week|month|year)s?\s*$/i', + '/\bago\b/i', + '/^in\s+\d+\s+(second|minute|hour|day|week|month|year)s?/i', + '/^(first|last)\s+day\s+of\b/i', + '/^(back|front)\s+of\b/i', + ]; + + foreach ($patterns as $pattern) { + if (preg_match($pattern, $trimmed)) { + return true; + } + } + + return false; + } + private function buildError(string $class, string $context): IdentifierRuleError { return RuleErrorBuilder::message("Avoid using new $class($context) directly. Prefer using \Psr\Clock\ClockInterface instead.") diff --git a/tests/Rules/Psr/Psr20RuleTest.php b/tests/Rules/Psr/Psr20RuleTest.php index 8fd8d26..e7842da 100644 --- a/tests/Rules/Psr/Psr20RuleTest.php +++ b/tests/Rules/Psr/Psr20RuleTest.php @@ -25,6 +25,25 @@ public function testRule(): void ["Avoid using new DateTime('now') directly. Prefer using \Psr\Clock\ClockInterface instead.", 6], ["Avoid using new DateTimeImmutable() directly. Prefer using \Psr\Clock\ClockInterface instead.", 7], ["Avoid using new DateTimeImmutable('now') directly. Prefer using \Psr\Clock\ClockInterface instead.", 8], + ["Avoid using new DateTimeImmutable('yesterday') directly. Prefer using \Psr\Clock\ClockInterface instead.", 10], + ["Avoid using new DateTimeImmutable('tomorrow') directly. Prefer using \Psr\Clock\ClockInterface instead.", 11], + ["Avoid using new DateTime('+1 day') directly. Prefer using \Psr\Clock\ClockInterface instead.", 12], + ["Avoid using new DateTimeImmutable('next Monday') directly. Prefer using \Psr\Clock\ClockInterface instead.", 13], + ["Avoid using new DateTimeImmutable('-2 weeks') directly. Prefer using \Psr\Clock\ClockInterface instead.", 14], + ["Avoid using new DateTimeImmutable('2 days ago') directly. Prefer using \Psr\Clock\ClockInterface instead.", 15], + ["Avoid using new DateTimeImmutable('last day of this month') directly. Prefer using \Psr\Clock\ClockInterface instead.", 16], + ["Avoid using new DateTimeImmutable('today') directly. Prefer using \Psr\Clock\ClockInterface instead.", 17], + ["Avoid using new DateTimeImmutable('friday') directly. Prefer using \Psr\Clock\ClockInterface instead.", 18], + ["Avoid using new DateTimeImmutable('in 3 weeks') directly. Prefer using \Psr\Clock\ClockInterface instead.", 19], + ["Avoid using new DateTimeImmutable('noon') directly. Prefer using \Psr\Clock\ClockInterface instead.", 20], + ["Avoid using new DateTimeImmutable('+0 seconds') directly. Prefer using \Psr\Clock\ClockInterface instead.", 21], + ["Avoid using new DateTimeImmutable('0 days') directly. Prefer using \Psr\Clock\ClockInterface instead.", 22], + ["Avoid using new DateTimeImmutable('yesterday noon') directly. Prefer using \Psr\Clock\ClockInterface instead.", 32], + ["Avoid using new DateTimeImmutable('today midnight') directly. Prefer using \Psr\Clock\ClockInterface instead.", 33], + ["Avoid using new DateTimeImmutable('tomorrow 12:00') directly. Prefer using \Psr\Clock\ClockInterface instead.", 34], + ["Avoid using new DateTimeImmutable('monday 14:00:00') directly. Prefer using \Psr\Clock\ClockInterface instead.", 35], + ["Avoid using new DateTimeImmutable('TOMORROW') directly. Prefer using \Psr\Clock\ClockInterface instead.", 36], + ["Avoid using new DateTimeImmutable('mon') directly. Prefer using \Psr\Clock\ClockInterface instead.", 37], ]); } } diff --git a/tests/Rules/Psr/fixtures/psr20.php b/tests/Rules/Psr/fixtures/psr20.php index aeec88f..d2efe0d 100644 --- a/tests/Rules/Psr/fixtures/psr20.php +++ b/tests/Rules/Psr/fixtures/psr20.php @@ -8,3 +8,31 @@ $d = new DateTimeImmutable('now'); $e = new DateTime('2023-01-01'); $f = new DateTimeImmutable('yesterday'); +$g = new DateTimeImmutable('tomorrow'); +$h = new DateTime('+1 day'); +$i = new DateTimeImmutable('next Monday'); +$j = new DateTimeImmutable('-2 weeks'); +$k = new DateTimeImmutable('2 days ago'); +$l = new DateTimeImmutable('last day of this month'); +$m = new DateTimeImmutable('today'); +$n = new DateTimeImmutable('friday'); +$o = new DateTimeImmutable('in 3 weeks'); +$p = new DateTimeImmutable('noon'); +$q = new DateTimeImmutable('+0 seconds'); +$r = new DateTimeImmutable('0 days'); +$s = new DateTimeImmutable('2023-12-31 23:59:59'); +$t = new DateTimeImmutable('2023-01-15T12:00:00+00:00'); +$u = new DateTimeImmutable('12:00:00'); +$dateString = '2023-01-01'; +$someVar = '2024-06-15'; +$args = ['2023-01-01']; +$v = new DateTimeImmutable($dateString); +$w = new DateTime($someVar); +$x = new DateTimeImmutable(...$args); +$y = new DateTimeImmutable('yesterday noon'); +$z = new DateTimeImmutable('today midnight'); +$aa = new DateTimeImmutable('tomorrow 12:00'); +$bb = new DateTimeImmutable('monday 14:00:00'); +$cc = new DateTimeImmutable('TOMORROW'); +$ee = new DateTimeImmutable('mon'); +$dd = new \stdClass();