Skip to content

[3.0] Checks that class names are written as ::class resolvers - #9666

Open
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:tests/class-name-syntax
Open

[3.0] Checks that class names are written as ::class resolvers#9666
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:tests/class-name-syntax

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Description

#9665 rewrote every class name SMF wrote down as a string into a ::class resolver. This is
the test that keeps it that way.

Nothing else can. phplint sees a valid string, php-cs-fixer does not read inside strings,
and PHP itself never resolves the name until the moment it is used — which for a background
task is a cron run on somebody else's forum, where TaskRunner::execute() logs the class it
could not find, drops the row from the queue and carries on. So the next one to be written
would sit there exactly as 'SMF\Tasks\FetchSMfiles' did, until somebody read the error log
of a forum they do not own.

The test walks Sources/, Themes/ and the five entry points, tokenises everything that
mentions SMF\ in a string at all, and fails on any string that spells out a class name:

Sources/TempProbe.php:11 names SMF\Tasks\CreatePost_Notify

with a message saying to write it as a resolver or to add it to the list of exceptions with
a reason.

The exceptions

Four names have to stay in a string, and the list says why for each:

  • ActionRouter asks whether its own class sits under 'SMF\Actions', which is a namespace
    and never a class.
  • Sources/Actions/Admin/PackageManager.php names the alias class_alias() is about to
    create. Writing the name being created as a resolver would claim the alias already exists.
  • Punycode probes for two Unicode helper functions, which only some data files define.
    Functions have no resolver.

The list is keyed by file and by the name itself rather than by line, so moving code around
a file does not invalidate it, and a second test fails if an entry names something that is
no longer there. An allowlist that outlives its reason is how a test like this stops meaning
anything, and I would rather it said so out loud than quietly wave through the next one.

The docblock on the list also heads off the entry somebody will reach for first: "the class
does not exist yet" is not a reason. ::class resolves at compile time and never asks the
autoloader, so it is perfectly happy to name a class that has not been loaded, or one that
is never loaded at all.

What it cannot see

A name assembled at run time. The menus build theirs as
__NAMESPACE__ . '\\Home::call', and the class half of that is not written down for
anything to check either — not by this test, and not by the compiler. Worth knowing before
reading a pass here as "every class name in SMF is checked".

Proving it fails

Both assertions were watched failing before being trusted: a throwaway Sources/TempProbe.php
holding 'SMF\Tasks\CreatePost_Notify' produced the failure above, and a bogus entry in the
exception list produced

Sources/ActionRouter.php no longer names SMF\NotThereAnyMore
Cost and its relationship to #9661

13.3s in the Docker environment. That is the bind-mounted filesystem rather than the work:
#9661's ClassReferenceTest, which walks the same tree, measures 12.1s in the same container
against the 0.151s quoted on that PR.

The two overlap, and reviewers may want only one. They answer different questions — #9661
asks whether a name written as a string is real, which still matters for the four above and
for anything a future change adds; this one asks whether SMF writes them as strings at all,
which after #9665 is the stronger statement. If both land they could share a single walk of
the tree. Happy to fold them together either way round.

Issues References (Fixes|Related|Closes)

  1. Related to [3.0] Writes class names as ::class resolvers rather than strings #9665 — this pins the rewrite that landed there.
  2. Related to [3.0] Checks that every class name written as a string names something real #9661 — overlapping coverage, discussed above.

🤖 Generated with Claude Code

A class name in a string is not a name PHP ever checks, and nothing in CI looks
inside a string, so the next one written that way would sit there until somebody
read the error log of a forum they do not own. This walks the tree and fails on
any string that spells out a class name.

Four names have to stay strings: a namespace prefix, the alias class_alias() is
about to create, and two Unicode helper functions. Each is listed with its
reason, and a second test fails if an entry names something no longer there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant