… if possible. In most cases it's possible to use the real object, and
reach into it's private parts via TestingAccessWrapper. This is almost
the same as using a mock, but I feel it's much more "light-weight".
The main change is that there is no strict assertion any more for the
number of ReferenceStack::pushInvalidRef() calls. Before this was mixed
into the same array as the valid references, as elements set to "false".
I think the test is as valueable as before without this extra check. If
the rollback stack works or not is already covered by other tests.
Change-Id: I90213557b164b3e43233a3dc393ee3f3d3d556a9
"Conflicting" here includes the case where one of two <ref> with the
same name does not have an extends attribute. The first occurence of
a name specifies if a <ref> is a top-level or a sub-reference. This can
not be changed later.
This patch changes multiple existing test cases. I checked all of them
in detail and confirmed the behavior is fine. The error reporting is
better or at least equally good in all cases.
Bug: T242141
Change-Id: Iaec306eefe5b168d496990105e297ca044a5e721
Allow a ref with `name=""` for backwards-compatibility.
Partially reverts I07738cce2641026dfaa92ba263ed6f9834be0944
Bug: T242437
Change-Id: Iaed2d1c41be377a4961aff39838b0965f6c00616
The goal of this patch is to not change any behavior, just make the
code less nested and less complicated.
Change-Id: I89170960ffbf61f57e245adf097f3e8d8196bbce
The difference between the two is that isOK() only reports "fatals",
while isGood() also reports "warnings" and "errors". I believe we
*want* to report all of these the same way.
Change-Id: I3be832c5db7aba3c03bd2ad8cfbba42362c093fd
A fun edge case where `name=""` fools both validation branches after
a references rollback, and triggered a LogicException. Stop these
freak refs.
Bug: T242437
Change-Id: I07738cce2641026dfaa92ba263ed6f9834be0944
It's possible to nest <references> by using tricky constructs like the
{{#tag function, and this breaks our rollback logic. Try to show normal
output, otherwise show an error.
Includes regression tests.
Bug: T242437
Change-Id: I33e497cdf8508ce7ccb7f0f315c00af5eee47d0e
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.FunctionComment.MissingDocumentationPrivate
* MediaWiki.Commenting.FunctionComment.MissingParamTag
* MediaWiki.Commenting.FunctionComment.ParamNameNoMatch
Additional changes:
* Also sorted "composer fix" command to run phpcbf last.
Change-Id: I148ec7fca3d53bddc2dc5abbcc1c229461feea33
This error happens only when previewing an edit, because some of the
validation in Cite::validateRefInReferences() is disabled in preview
mode. Unfortunately this codebase was never properly tested in preview
mode.
This patch is intentionally so small to make it easy to backport.
Tests will follow.
Bug: T242434
Change-Id: I5e529b7227598ab2acc624c90a0cb5d09b3f5452
I would like to argue that calling the current state of this codebase
"version 1.0.0" is plain wrong. A more meaningful version number can
easily be introduced later if one is needed.
Bug: T213066
Change-Id: I4854263592784feb072acea2d9efe99f1f04ab28
Each of these TODOs is something that needs to be fixed or implemented,
so it's helpful to map them to tasks.
Change-Id: I807208392d8a609d7f3b371dc3560a48f3578092
* Always have an empty line between @param and @return to improve
readability as well as consistency within this codebase (before, both
styles have been used).
* Flip parameter order in validateRefInReferences() for consistency with
the rest of the code.
* In Cite::guardedRef() the Parser was now the 1st parameter. I changed
all related functions the same way to make the code less surprising.
* Same in CiteUnitTest. This is really just the @dataProvider. But I feel
it's still helpful to have the arguments in the same order everywhere, if
possible.
* Add a few strict type hints.
* It seems the preferred style for PHP7 return types is `… ) : string {`
with a space before the `:`. There is currently no PHPCS sniff for this.
However, I think this codebase should be consistent, one way or the other.
Change-Id: I91d232be727afd26ff20526ab4ef63aa5ba6bacf
There is currently no strict CodeSniffer rule for this. I think we
need to have one sooner or later. Anyway, what I find important is to
have a consistent code style in one codebase.
I refused to do this change previously because I don't like to mess
with Git blame if it's not really necessary. However, at this point all
code was moved around anyway.
I ended removing a comment that appears misplaced now, and doesn't help
maiing the code more readable. I like not having a dot at the end if
it's not really a sentence.
Change-Id: Id1d4f43277c69080c512c1a5ceff4c948bfa05be
In the end I don't care much if we agree on having this newline, or
not. What I care about more is that this codebase is consistent.
Personally I prefer having the newline. It creates a visible separation
between what "goes in" and what "goes out" (@throws and @return).
Change-Id: Ibc60af621132e415a5579397c01688fa21eb0be5
The rollback feature was not able to properly restore a __placeholder__.
That's why a specific use case was behaving different. This already
worked just fine:
<ref extends="a">…</ref>
<references>
<ref name="a">…</ref>
</references>
But this didn't, even if it is the exact same from the users
perspective:
<ref extends="a">…</ref>
{{#tag:references|
<ref name="a">…</ref>
}}
Bug: T239810
Change-Id: I163a1bffb9450a9e7f776e32e66fb08d0452cdb9
