Implicitly marking parameter $... as nullable is deprecated in php8.4,
the explicit nullable type must be used instead
Created with autofix from Ide15839e98a6229c22584d1c1c88c690982e1d7a
Break one long line in SpecialPage.php
Bug: T376276
Change-Id: I807257b2ba1ab2744ab74d9572c9c3d3ac2a968e
Use
$request->getRawVal( 'key' ) ?? 'default'
instead of
$request->getRawVal( 'key', 'default' )
The ?? is more flexible, avoids a wrong type detection by phan and
avoids the evaluation of the default value if not needed.
Bug: T376245
Depends-On: I3ed6b85c0d117ed7cb3a8b79f73a3eb42977891e
Change-Id: I8b02f9297b76d04e21f8cb9194f3b85631956eca
This avoids addition of new code with the deprecated global,
or at least it gives extra attention on review when new code also uses
the inline ignore
Change-Id: I5c1bc5a1685c28f153d4fbe3525959930f54b557
Providing the function name is often optional from the php code,
but it is needed for better logging, so make it mandatory and let phan
report issues about this.
Bug: T374546
Depends-On: Iaed5489a85a5a6e685829e151436afc94310fbd0
Depends-On: Ie2a1e5052e5b61bbb5b89905de942f47d3f1413d
Change-Id: I5227f2fa65850ac8c6f620900f22d1f4e7bfd470
In change I625a48a6ecd3fad5c2ed76b23343a0fef91e1b83 I am planning to
make Wikimedia\Message\MessageValue use it, and we try to pretend that
it is a library separate from MediaWiki, so it makes sense to move
MessageSpecifier to the same namespace under Wikimedia\.
Bug: T353458
Change-Id: I9ff4ff7beb098b60c92f564591937c7d789c6684
Viewing a page which transcludes Special:Prefixindex leads to the
default skin being used for the main request, instead of the user's
skin, in a regression of a bug from 2023. Tracing shows that
OutputPage::setupOOUI() calls RequestContext::getSkinName(), and the
skin name is permanently cached.
So, reset the skin name when the user is reset. This is a simple partial
fix.
Bug: T336504
Change-Id: If630f337ba6fb2b889e657d49112b878c3ad5f83
* Switch out raw Exceptions, mostly for InvalidArgumentExceptions.
* Fake exceptions triggered to give Monolog a backtrace are for
some reason "traditionally" RuntimeExceptions, instead, so we
continue to use that pattern in remaining locations.
* Just entirely give up on PostgresResultWrapper's resource vs. object mess.
* Drop now-unneeded false positive hits.
Change-Id: Id183ab60994cd9c6dc80401d4ce4de0ddf2b3da0
The function is not defined in the extends interfaces to inherit
documentation from.
Follow-Up: I8334cc89dcf0f293298b82e004116be50a90f0d1
Change-Id: I14b2adfaa3c859ce7b196fc639b0974bbbf01834
The idea is that all formatters that need the user language or
other request specific context should be instantiated by
FormatterFactory.
Change-Id: I8334cc89dcf0f293298b82e004116be50a90f0d1
Similiar to the reset in setUser() from 242a5b2a
Just in case as more code could be migrated to use setAuthority()
Follow-Up: I4fc815f00933193f6b0eeb996a2f75a4295ecb01
Change-Id: I0aa6c3de5413ecf66b5b21f5229dd3026ff9f118
When $wgHiddenPrefs['skin'] = true and $wgDefaultSkin is set to a
nonexistent skin, SkinFallback should be used. An exception should not
be thrown.
An odd case, but it did work before a99e31533b.
Add test. Simplify the existing tests, using StaticUserOptionsLookup
instead of the mock builder.
Bug: T342733
Change-Id: I372ace56c104a3ecc9c02d87524e8ef0be7239cf
I had to read this 5x to convince myself that it wouldn't discard
the assigned `$this->skin` object (from the elseif branch in the middle)
in the `if ( $skinName !== null )` branch below.
It appears to work fine, but it may be clearer to avoid the indirection
through checking that $skinName got assigned and rather move those
branches together.
This coincidentally matches the evaluation order of fetchSkinName().
Change-Id: Ia20136b8f04c8e8d89d29b97cdd99cb444c09838
There are about 33 callers in Code Search, and almost all were calling
it with incorrect parameters.
It was incorrect to call it with no parameters, since this would
globally set the wrong skin and direction. Most callers were doing this.
It was incorrect to call it with the values from some local context.
We are setting a global singleton here so only the global RequestContext
has the correct values.
Usually, setupOOUI() is called to avoid the exception which is thrown
when HTML is generated without a singleton. But it was often called
multiple times in a request, because different parts of the stack need
OOUI HTML and they don't know whether OOUI has been initialised.
So, ignore the parameters passed to setupOOUI(). Ignore all calls after
the first one, so that we can stop throwing away the theme object. Reset
the cached data when RequestContext::setSkin or
RequestContext::setLanguage() are called.
Add RequestContext::getSkinName() so that the name of the skin can be
determined without actually creating the skin object. It would be
complicated to add the method to IContextSource since there is a
method of the same name in Skin, which implements IContextSource.
Change-Id: I7a1e8b613408d2ecc2e497e0672644af85ce0da3
"BadMethodCallException" sounds like it would fit, but it does
have a very different meaning, described as "exception thrown if
a callback refers to an undefined method or if some arguments are
missing". This is not what's going on here. These are methods that
should only be called from unit tests.
This appears to be a common mistake, often copy-pasted.
Change-Id: Ib39e28f596a883481d5f526460a5c871c75f5313
Make it extend SpecialPageTestBase and simplify it accordingly, removing
a lot of things that are no longer needed. I'm not even sure if this
test still serves a purpose, but at least it should be easier to
maintain now. This also fixes a test failure when
UniversalLanguageSelector is enabled due to a ULS hook error.
This approach still needs a hack for overriding UserOptionsManager,
because it's used in a lot of places with lots of expectations and
global state, and the mock would need to be much more complex for that.
Also:
- Remove unchecked exceptions from doc comments.
- Fix indentation of a conditional where the second line of the
condition was indented the same as the body, which made it hard to
read.
- Add some return typehints to various methods. These make it easier to
write tests because if a method is return-typehinted, PHPUnit will
mock the return value automatically, instead of returning null and
leaving it up to the developer to provide a mock explicitly.
- Add string typehint to Skin::normalizeKey. The value is already
assumed to be a string, and passing null to strtolower emits a
deprecation notice in PHP 8.1.
Change-Id: I80723b886b2b5a5d75cbb73571e1b19ea4a09af5
The Hooks class contains deprecated functions and the whole class is
going to get removed, so remove the convenience function and inline the
code.
Bug: T335536
Change-Id: I8ef3468a64a0199996f26ef293543fcacdf2797f
* Illegal string offset and invalid argument supplied to foreach, due to incorrect type information
* Array internal pointer reset is unnecessary
* $hookData unused since MW 1.35 due to incomplete revert
* array_push() with single element
* Unnecessary sprintf()
* for loop can be replaced with str_repeat()
* preg_replace() can be replaced with rtrim()
* array_values() call is redundant
* Unnecessary cast to string
* Unnecessary ternary. Often the result relies on short-circuit evaluation, but I find it more readable nonetheless.
Change-Id: I4c45bdb59b51b243fa96286bec8b58deb097d707
This reverts dc3bd3d721 (I385dca1d95) and re-applies d4ce0f3255 (Ib9fc34ca64).
The CI failures have been addressed.
Bug: T314008
Change-Id: I35a4f656c31b67ebb662bf6f6366f4ee846ecbda
I think it's within the meaning of the method to sanitize the type of
the input. Code review shows that it's almost always called with
technically nullable input, so it's convenient to deal with nulls at
this single place.
The linked bug is a PHP 8.1 deprecation warning due to a test which set
up a StaticUserOptionsLookup without a language option.
StaticUserOptionsLookup does not fall back to the site defaults, so the
language was null, causing an error from strtolower().
Bug: T322099
Change-Id: I6dc61476c6869a7648a67be79a4835a5ac24fa92
Follows similar commits to the objectcache, rdbms, filerepo,
jobqueue components and other areas [1].
* Remove duplicate descriptions from file blocks in favour of class
doc blocks. This reduces needless duplication and was often
incorrect or outdated, and helps (ironically) to make the file header
more consistently visually ignorable.
* Remove `ingroup` from file blocks in class files as otherwise
the file is indexed twice (e.g. in Doxygen) which makes navigation
more messy.
* Fix non-standard `@unstable for implementation` annotations
in favour of `@stable to type` as per T257789 and
<https://www.mediawiki.org/wiki/Stable_interface_policy>.
While at it, fix the only other outstanding uses of `@stable for`
in core as well in a handful of context/, logging/ and search/
files.
[1] https://gerrit.wikimedia.org/r/q/message:ingroup+owner:Krinkle
Bug: T257789
Change-Id: Ided3c5ab69e1b587b1b76a3c97a7cdb88f21e130
