refactor(forum): drop four dead locals and initialise $tracktext - #5876
Merged
Conversation
None of these four variables is read after it is written: - e107forum::__construct() takes a parser it never uses. - e107forum::getTrackedThreadList() takes an e107 instance it never uses. - plugin_forum_view_shortcodes::sc_track() globals $forum, which appears nowhere else in the method outside a commented-out block. - forum_front::forum_track() captures getTrackedThreadList()'s return into $trackedThreadList purely to test it. The call is kept; only the unused assignment goes. $tracktext is initialised alongside it because it is assigned only inside that branch and concatenated unconditionally afterwards, so a user with no tracked threads reached an undefined variable. The rendered output is unchanged: the branch already produced an empty string's worth of markup in that case, it just warned on the way. Split out of the pure-cleanup portion of #5320 by rica-carv, at Deltik's request. The deprecations and the redirect in that PR are unrelated to these and stay there for discussion. Co-authored-by: rica-carv <rica-carv@users.noreply.github.com>
Deltik
approved these changes
Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #5320 at @Deltik's request (comment): this carries only the pure cleanup, so it can land while the rest of that PR is discussed.
What's here
Four variables that are written and never read:
forum_class.php#L106$tp = e107::getParser();ine107forum::__construct()forum_class.php#L1141$e107 = e107::getInstance();ingetTrackedThreadList()view_shortcodes.php#L1073global $forum;insc_track()forum.php#L420$trackedThreadList =inforum_track()In the last one only the assignment goes; the
getTrackedThreadList()call still runs and still gates the branch.Plus one initialisation that belongs with them.
$tracktextis assigned only inside that branch (forum.php#L500) and concatenated unconditionally afterwards (#L508), so a user with nothing tracked hit an undefined variable on PHP 8. Setting it to''up front removes the warning without changing a byte of what renders.$tpand$sqlinforum_track()are both genuinely used, so they stay.What's deliberately not here
The rest of #5320 is not cleanup and shouldn't ride along:
e107::redirect(); exit;when nothing is tracked.HEADERFis included atforum.php#L106, two lines beforeforum_track()is called at#L108, so headers are long gone by then.e107::redirect()also defaults to 301 (e107_class.php#L4902), which a browser would cache. The empty-state handling is worth fixing, just not that way.{THREADTITLE},{REPLYTITLE},{LASTPOSTITLE},{INFOTITLE},{NEWTHREADTITLE},{POSTEDTITLE},{TRACKTITLE}) in favour of{LAN=...}. That's a real BC decision: any theme carrying its own forum template with those tokens goes blank. Needs a deprecation path, not a commented-out method.{ICONKEY}templatisation and the changes tosc_topic_lastpost_date/sc_topic_lastpost_author, which alter what those shortcodes return in the no-replies case.Those stay on #5320 for @rica-carv and me to work through.
Testing
Syntax-checked on PHP 8.5. No behavioural change to verify: every removal is of a value nothing reads, and the
$tracktextinitialisation only replaces an undefined-variable warning with the empty string PHP was already coercing it to.Original work by @rica-carv in #5320; credited via
Co-authored-by.