Commit graph

309 commits

Author SHA1 Message Date
jenkins-bot 99d5ad6d36 Merge "eslint: Remove unused rules" 2020-04-25 18:50:07 +00:00
Ed Sanders bd8010c360 eslint: Remove unused rules
Change-Id: Iaba2e171da4a969454de1006883a5320a402d9c8
2020-04-24 22:19:27 +01:00
C. Scott Ananian 376c0418d3 Update parser tests to v2 (tidy by default)
The most common cleanup required by switching to tidy output was adding
missing <p>-wrappers to the last item before <references/>.

Bug: T246285
Change-Id: I7c8a08c4e6eff7caf4539a26fae475a4133f9a0c
2020-04-01 11:11:13 -04:00
Thiemo Kreuz a00b8d990d Add test cases for impossible follow vs. rollback edge case
While working on the patch I4303642 I was worried about the line

array_pop( $this->refCallStack )

in the rollback code. Since the patch changed the position of follow
elements in the stack, an array_pop() would pop different elements.

It turns out this is impossible. Rollbacks are only done for <ref>
elements inside a <references> tag, immediatelly after reaching the
closing </references>. It's impossible to use follow="…" inside
<references>. It will not be added to the stack, and therefore not
rolled back.

Even if the edge case would be possible, the *old* code that placed
follow elements on the *other* side of the stack would have been
wrong then.

The test cases in this patch try to hit this edge case, and are
expected to not be able to do so.

Change-Id: I4380bf443db17c6214dbfa2cbda62b46db04258a
2020-04-01 09:03:19 +00:00
jenkins-bot a27b4c82a6 Merge "Revert "Add a newline in wikitext before autogenerated reflist"" 2020-04-01 08:15:41 +00:00
Awight 8c933d03c5 Revert "Add a newline in wikitext before autogenerated reflist"
This reverts commit 90697ffe43.

Change-Id: I659ce1689603fd16e378fb8d3d5bd6d1089342b2
2020-04-01 08:03:55 +00:00
jenkins-bot 45f4990e34 Merge "Add a newline in wikitext before autogenerated reflist" 2020-03-31 19:37:34 +00:00
Bartosz Dziewoński 90697ffe43 Add a newline in wikitext before autogenerated reflist
Previously the reflist was added at the end of the last line of text,
which messes up paragraph wrapping (as seen in many test cases), and
generated invalid HTML when the last line was a list item (T148701).

Bug: T148701
Change-Id: Ifc873fc913e717026d80d54b570c594d1073fb42
2020-03-31 19:00:51 +00:00
jenkins-bot a00401fdfc Merge "Remove not needed code without changing anything" 2020-03-31 18:30:21 +00:00
Thiemo Kreuz 53b043f28f Remove not needed code without changing anything
This removes a few tiny pieces of code, and a large chunk related to
incomplete follow="…" attributes (see T240858). It turns out we don't
need to insert elements at the top of the ReferenceStack::$refs
array, because this array is reordered anyway in
ReferencesFormatter::formatRefsList()!

Incomplete follow refs don't have a number, and are ordered to the top
because of this, as before. This doesn't change with this patch.

Change-Id: I43036420be22feb8f0f287d9ccee2afd317df2a9
2020-03-31 18:15:14 +02:00
Timo Tijhof ce27a400e1 CiteHooks: Remove wgResourceModules check (redundant with isModuleRegistered)
The isModuleRegistered() method was introduced a few years ago,
when the load order in ResourceLoader was undergoing a change.

It used to be that hooks like were run first to register modules, and then
wgResourceModules was registered afterwards. This was reversed to disallow
mutating the config at run-time from foreign modules and to allow better
caching and error detection.

It's been several years since then, so this redundant check is no longer
needed. ServiceWiring.php in MW core for ResourceLoader always processes
config and extension.json first before this hook is called.

Bug: T247265
Change-Id: I466f1fa70b8f0e9fe5e8e8df90bb0001b3329b87
2020-03-10 16:18:46 +00:00
Brian Wolff 5ed973a49a Fix unit tests for whitespace change in Html5 fragments
Bug: T238385
Depends-On: Ie2b7c9429691e2c491c3359d5b400d8f078aa789
Change-Id: Ie2b7c9429691e2c491c3359d5b400d8f078ab111
2020-02-25 13:56:53 +00:00
Brian Wolff 1c6a92ca9b Temp disable test to work around circular dependency in unit test
Html5 fragment mode now bans whitespace per html5 spec

See Ie2b7c9429691e2c491c3359d5b400d8f078aa789

Change-Id: Ie6fa40798f06a358f6082110b4d8cc0028c80321
2020-02-25 04:38:06 -08:00
jenkins-bot 7ee2e81ade Merge "Add two extreme follow edge cases back to parser tests" 2020-02-11 09:11:20 +00:00
Thiemo Kreuz 400ce89f30 Don't talk about follow being "broken" but "incomplete"
Bug: T240858
Change-Id: Iab6563fdf19d6e85795911e4140476fceabf7334
2020-02-05 16:38:49 +00:00
Thiemo Kreuz 48e2f02e20 Add two extreme follow edge cases back to parser tests
This reverts parts of the revert I3bee35f, which reverted a3d312c8.
I believe it's helpful to keep these test cases just to document how
the code currently behaves. I removed all TODO because we don't know
if and how we want to touch this again.

Bug: T240858
Change-Id: Ib91acfcb7292e5c03ce9cc4d7be782085e10aa27
2020-02-05 15:04:49 +01:00
Adam Wight f2bd6b6dcc Revert "Standardize "follow" validation"
This reverts commit a3d312c8f4.

Bug: T240858
Change-Id: I3bee35f27797a04c41c265f7e598d8383414b67a
2020-02-05 11:42:28 +01:00
Adam Wight b15f1b81a0 Revert "Remove "follow" special case from ReferencesFormatter"
This reverts commit 38122d91cd.

Bug: T240858
Change-Id: I7198d5534acded94bc83962262c4cdfed9782454
2020-02-05 11:42:27 +01:00
Adam Wight eb799af3df Revert "Remove broken "follow" special case from ReferenceStack"
This reverts commit d01cba60fb.

Bug: T240858
Change-Id: I5b528a285ed6a658ceb333b58f0f4a81a64c7f15
2020-02-05 11:42:26 +01:00
jenkins-bot 7c3ace4fef Merge "Rewrite ErrorReporter for performance and separation of concerns" 2020-02-03 14:58:55 +00:00
jenkins-bot 74fab9755f Merge "Remove one unnecessary LogicException from ReferenceStack" 2020-02-03 14:50:40 +00:00
Thiemo Kreuz d80bd3ef97 Rewrite ErrorReporter for performance and separation of concerns
This patch is mostly moving code around without changing the behavior.
Exceptions:

* The ErrorReporter creates a <span> container. This was previously
parsed. The only benefit might be error checking and escaping. Rather
pointless. The code just created this HTML. With this patch, it is not
parsed any more. The unit test reflects this change. The output in
production will not change, as the parser tests show.

* Parsing of the message key (to detect it's type and id) is simplified
a lot, using explode. With this the code can, in theory, support more
types.

Bug: T239572
Change-Id: If2fe5f55db46dfc7e0ce445348608bef00bec64e
2020-02-03 15:23:40 +01:00
Adam Wight d01cba60fb Remove broken "follow" special case from ReferenceStack
This is unreachable now that broken follow refs fail validation.

Bug: T240858
Change-Id: I22adaee9c4eaeb94bee953ae15c642e044b6a54b
2020-02-03 12:27:59 +01:00
Adam Wight 38122d91cd Remove "follow" special case from ReferencesFormatter
This is unreachable, now that broken follow refs fail validation.

Bug: T240858
Change-Id: I77faeaac4bc53632ab8b82bff7e335ee8c99dfa5
2020-02-03 12:27:57 +01:00
Adam Wight a3d312c8f4 Standardize "follow" validation
Perform the validation in validateRef, and display a new error message for
broken "follow" refs.  This changes existing behavior, where broken folow
ref content is arbitrarily displayed at the top of the references list and
no error is rendered.

Thanks to weasely wording, the new error can later be reused for "extends"
errors.

Bug: T240858
Change-Id: I506e4dcd1151671f5302ecd99581145d979d8124
2020-01-30 17:25:42 +00:00
Thiemo Kreuz 0fda08b25a Remove one unnecessary LogicException from ReferenceStack
This exception was introduced very late in the patch I38c9929. It
already caused trouble. This here is essentially a revert. It restores
the previous behavior where this edge-case was silently ignored. The
worst thing that can happen is that appendText() creates an incomplete
entry in the $this->refs array, which will be rendered at the end. The
user can see it then.

As of now we are not aware of a code path where this would even be
possible. Still this does make the code *more* robust by not making it
explode, but give the user something they can work with.

Bug: T243221
Change-Id: I2e2d29bbd557090981903fcc2ece8796fafa4aa4
2020-01-28 16:15:55 +01:00
jenkins-bot 3c8a225052 Merge "Fix incomplete rollback producing bad footnote numbers" 2020-01-24 15:11:31 +00:00
Thiemo Kreuz 2ddc6f133b Fix incomplete rollback producing bad footnote numbers
Bug: T48140
Change-Id: I53ce5d8488d4c24d6f23f6f0e70806d7db4064e1
2020-01-24 13:02:53 +01:00
Thiemo Kreuz 816b1b0add Remove newline characters from all error messages
These create bogus output, depending on the surrounding wikitext the
<ref> tag is used in. For example, this example wikitext:

* Example.<ref name="1">a</ref> More text.

… will be rendered with the "More text" sentence wrapped on the next
line, outside of the list. However, this does *not* happen in many of
the localizations, e.g. German, because many Tanslatewiki translators
did not copied the bogus \n. Why should they.

TL;DR: These newline characters either do nothing, or destroy the output.
In both cases the proper fix is to replace them with spaces.

Some of the test cases touched in this patch demonstrate the issue.

Change-Id: I395a40637a5293eda1f477963d252ce1a215f8b2
2020-01-24 12:29:14 +01:00
Thiemo Kreuz 51d55bb8de Introduce dedicated error message for nested <ref extends=…>
This resolves another TODO. Since this is an intentional limitation in
the design of the feature, I find it pretty signigicant to give it it's
own error message.

Note that the text does not need to be perfect, just good enough for now.
We will review all error messages later via T238188.

Bug: T242141
Change-Id: Id9c863061e855350320131e81f6702c8810736f4
2020-01-23 15:00:26 +01:00
jenkins-bot 84341c3603 Merge "Replace ReferenceStack mocks with actual instances" 2020-01-21 11:16:30 +00:00
Thiemo Kreuz 9565d6e887 Resolve a TODO by covering it with a test case
It turns out this is indeed necessary. The test demonstrates why.

Change-Id: Id9c6a48f72ef8d3f0cc9a714d826418e69913b0a
2020-01-21 10:11:03 +00:00
jenkins-bot 8700177736 Merge "Use StatusValue::isGood() instead of isOK()" 2020-01-20 16:26:29 +00:00
Thiemo Kreuz 6a4a0fd013 Replace ReferenceStack mocks with actual instances
… 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
2020-01-20 16:31:48 +01:00
Thiemo Kreuz d8651dda81 Clean up mocks and assertions in ErrorReporterTest
For example, there is no need to create a mock in a callback.

Change-Id: I8879b662ac69ba62fe9c0eb86f592493065e24b1
2020-01-20 14:50:30 +01:00
jenkins-bot 258b23a6dd Merge "Error when reusing <ref> with conflicting "extends" attributes" 2020-01-20 13:49:41 +00:00
Adam Wight 8a58ed55dc Error when reusing <ref> with conflicting "extends" attributes
"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
2020-01-20 13:33:52 +00:00
Adam Wight b3ea9f4ef8 Relax empty-string name validation
Allow a ref with `name=""` for backwards-compatibility.
Partially reverts I07738cce2641026dfaa92ba263ed6f9834be0944

Bug: T242437
Change-Id: Iaed2d1c41be377a4961aff39838b0965f6c00616
2020-01-20 12:40:09 +00:00
Thiemo Kreuz b78d85e728 Use StatusValue::isGood() instead of isOK()
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
2020-01-20 12:35:48 +01:00
jenkins-bot 1ca0905b98 Merge "Fix PHPUnit 8 warning" 2020-01-20 11:06:37 +00:00
Max Semenik 9ecf523eee Fix PHPUnit 8 warning
Bug: T192167
Change-Id: I78ef2aa360e71a5fe214c54807aaa4afbb40c026
2020-01-20 10:33:56 +00:00
jenkins-bot f2cda50778 Merge "Add unit test for section preview regression" 2020-01-20 10:08:43 +00:00
Arlo Breault 7e2bbae4c2 Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 0a6c576ad6ccfc81c2bfa20757417c62e554ef56

Change-Id: Ifa5d60e362e5c530d12d3b94351aef2d1b1962cc
2020-01-17 14:51:03 -05:00
Adam Wight f3031b80b9 Fix for blank-named ref in #tag
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
2020-01-17 11:19:29 +01:00
Adam Wight 1c947a808d Fix for nested #tag:references
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
2020-01-15 12:44:29 +01:00
Thiemo Kreuz ceb3a1ed5f Add unit test for section preview regression
Bug: T242434
Change-Id: I3e87897a1f9f418c4dd72d3137c74340b6646930
2020-01-14 15:10:47 +01:00
jenkins-bot 09f4deede4 Merge "Replace now unused native cloning feature" 2020-01-09 14:13:58 +00:00
jenkins-bot d18fbcffef Merge "Rewrite ReferenceStackTest::provideRollbackRefs for readability" 2020-01-09 12:53:34 +00:00
jenkins-bot ad0c94bf22 Merge "Annotate TODOs with task number" 2020-01-09 12:48:49 +00:00
jenkins-bot 6d02c1569d Merge "Final clean-ups for a more consistent parameter order" 2020-01-09 12:44:55 +00:00
Adam Wight 170484e933 Annotate TODOs with task number
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
2020-01-09 13:13:48 +01:00
jenkins-bot 0be582dc12 Merge "Report conflicting extends="…" with an error message" 2020-01-09 11:37:27 +00:00
Thiemo Kreuz 013e1bfa90 Final clean-ups for a more consistent parameter order
* 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
2020-01-09 12:13:54 +01:00
jenkins-bot b9b6905171 Merge "Fix incomplete undo/redo stack implementation" 2020-01-09 10:58:35 +00:00
Thiemo Kreuz 04fbbbd3ca Report conflicting extends="…" with an error message
Bug: T242110
Change-Id: I04342b2c219981dfb9575ea58cfccf6c2ba1066c
2020-01-08 16:47:07 +00:00
Thiemo Kreuz d07110b790 Fix incomplete undo/redo stack implementation
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
2020-01-08 17:43:02 +01:00
Adam Wight b7c9dbb0d5 Remove invalid test case
Unnamed references are never merged.

Bug: T239788
Bug: T240459
Change-Id: I8dd3706c688108bf2e3c0e9b55f123084b325d16
2020-01-08 16:59:28 +01:00
jenkins-bot 861c4edba7 Merge "Test cases for extends pointing to the <references> section" 2020-01-08 10:42:14 +00:00
Thiemo Kreuz 6ddfd9983b Fix bad numbering when reusing sub-references
Note this leaves *another* bug behind. When a <ref> is properly reused
by name="…", and the content is fine (either missing or identical),
possibly conflicting extends="…" attributes are currently entirely
ignored. However, this is already much better than what happened before.

Bug: T242110
Change-Id: Id808ce31c8036cc290f68bb3e8c5a7b12f4f44cf
2020-01-07 16:34:05 +01:00
Thiemo Kreuz 5db90fb5a9 Test cases for extends pointing to the <references> section
This is an extremely relevant use case, but we never had a test for
this:

Some text.<ref extends="book">Page 2</ref>

<references>
  <ref name="book">Title of the book</ref>
</references>

What this means: There is no reference in the text that points to the
book as a whole, only references that point to individual pages. The
base <ref> is not used in the text.

This is already properly rendered. There is no "jump back to the text"
link. However, this fails when <references> is wrapped in {{#tag:…}}.

Bug: T239810
Change-Id: Id22db0238266a4fd6131d1a10eb6bf6227552c19
2020-01-07 12:43:18 +01:00
jenkins-bot 44f3f5bf44 Merge "build: Updating mediawiki/mediawiki-phan-config to 0.9.0" 2020-01-07 03:47:57 +00:00
Thiemo Kreuz 38d5bd5f39 Add missing parser tests for relevant responsive edge cases
I tried to run these tests with a very old version of this code base
(from 2018) to confirm this is the correct behavior.

Bug: T241303
Change-Id: Id97d016b199458aa178ca732282e9c0e91e291a4
2019-12-28 20:59:23 +00:00
libraryupgrader 2e0792a0dd build: Updating mediawiki/mediawiki-phan-config to 0.9.0
One of the most significant changes is when I noticed that the $group
can never be null. We set it to DEFAULT_GROUP before. That's an empty
string.

I'm not very happy with the two @phan-suppress-next-line. Is there a
better way to fix these lines?

Change-Id: I33c1681e2f3857cb6701da71f4ed8893caff4d1e
2019-12-27 19:45:17 +00:00
Thiemo Kreuz ed5d72456d Rewrite ReferenceStackTest::provideRollbackRefs for readability
I hope this is more readable. This patch does two things: It uses
array keys to name all elements in the data provider. (Note these
array keys don't actually do anything, PHPUnit ignores them.) And this
patch merges two parameters into a single $expectedResult.

Change-Id: Ib7adc32bf8bfd523735591d35d0bcabd3b853cfc
2019-12-24 14:31:27 +01:00
Thiemo Kreuz 0dc6f37785 Replace now unused native cloning feature
Since I3db5175 the ParserCloned hook handler does not rely on cloning
the Cite object any more. There is no cloning any more. This is dead
code and we could remove it. Just to be sure I propose to keep the
method, but let it throw an exception.

Bug: T240248
Change-Id: I2057ea652ca25f4c7031c28a6e713671738f5e22
2019-12-20 20:07:59 +01:00
Adam Wight 2a3879eafa Harden logic assertions
These should be impossible conditions, we don't want to continue with
processing.

I hate this patch, it's a temporary workaround until someone rewrites
or replaces the rollback logic, for example with a two-pass parse.

Change-Id: I6a1327e397d4272fa412c3f290c2107d867d2854
2019-12-20 14:53:29 +01:00
Thiemo Kreuz 028424a682 More function call argument unpacking
I hope this patch is not to horrifying and can be reviewed. It's
possible to split this into a sequence of smaller patches. Please
tell me.

Change-Id: I4797fcd5612fcffb0df6c29ff575dd05f278bd4d
2019-12-19 12:58:02 +01:00
jenkins-bot 347ad9fb5f Merge "Change order of elements in the refs call stack" 2019-12-19 10:30:35 +00:00
jenkins-bot 45119f8c61 Merge "Move "dir" error handling to validation" 2019-12-19 10:18:24 +00:00
jenkins-bot 0394cd8599 Merge "Add missing @covers tags to tests" 2019-12-19 10:13:56 +00:00
Thiemo Kreuz 38fe3665e5 Change order of elements in the refs call stack
The main benefit is this nifty call: `$this->rollbackRef( ...$call )`

To make this possible, the minimal change I needed to do was to move
the two $argv and $text arguments to the end.

I also tried to order all other arguments as good as I could: Required
first, optional later. Group and name together. Name and extends
together.

All this is private implementation and should not affect anything.

Change-Id: I7af7636c465769aa53122eb40d964eabdd1289ba
2019-12-19 09:27:15 +01:00
Thiemo Kreuz 5c65525c95 Introduce ReferenceStack::appendText
I feel this is a little better than before. It looks like we never need
to *replace* a text that existed before.

This depends on I4a156aa which fixes one of the last remaining trimming
issues. Outside of <references>, a <ref> </ref> with no other content
but some whitespace was already forbidden. But not inside of <references>.
This is relevant for appendText(). It should not be called with null, but
was because of the inconsistent behavior.

Change-Id: I38c9929f2fa6e69482e45919e2f8dbf823cb1c8b
2019-12-19 08:52:48 +01:00
Arlo Breault 6d55f9e8cc Sync up with Parsoid citeParserTests.txt
This now aligns with Parsoid commit 41f397ce4d563fa7f7770725d88944dcabda4116

Change-Id: I27b7f035c8b99ca80501b8cd1169ed8c8895ef93
2019-12-18 15:30:49 -05:00
Thiemo Kreuz 92607eecfd Add missing @covers tags to tests
We forgot about these when restructuring the code and introducing these
new methods.

Change-Id: If856a7857f2d50d1dc66a57999d98baa8b924d51
2019-12-18 16:46:15 +00:00
jenkins-bot 0d7e04e1ee Merge "Fix inconsistent error reporting for invisible content" 2019-12-18 09:27:03 +00:00
Adam Wight 1e82f8f073 Move "dir" error handling to validation
Note that this patch changes behavior, an invalid "dir" will result in
a cite reference at the point where the <ref> is declared rather than
in the references section.  This is consistent with other errors.

Bug: T15673
Change-Id: Id10db40aa0b391f2f1d9274aa09d22a7278d65e3
2019-12-18 10:05:59 +01:00
jenkins-bot ce14db4048 Merge "Add parser tests for the responsive="…" feature" 2019-12-18 07:10:22 +00:00
Thiemo Kreuz 1f76199ed8 Add parser tests for the responsive="…" feature
Change-Id: Id9d733dabf82f2c26f51c6fbd1e03fe0574e88a8
2019-12-17 15:51:41 +01:00
Thiemo Kreuz e5cda65fbe Remove single use classes from the use section
The name of the base class in tests is guaranteed to only occur a
single time in a file. There is not much value in making it relative,
and requiring it to appear in the use section. Especially because it
is in the root namespace.

This reflects what I once encoded in the sniff
https://github.com/wmde/WikibaseCodeSniffer/blob/master/Wikibase/Sniffs/Namespaces/FullQualifiedClassNameSniff.php
I wish we could pick this rule and use it in our codebases. But it
seems it is to specific and can't be applied on all codebases, hence
it can't become part of the upstream MediaWiki rule set. At least not
at the moment.

Change-Id: I77c2490c565b7a468c5c944301fc684d20206ec4
2019-12-17 14:57:55 +01:00
Thiemo Kreuz 1bd66081f7 Fix inconsistent error reporting for invisible content
This makes one of the last remaining edge-cases about non-empty, but
non-visible content (a <ref> that only contains whitespace) behave
identical to all other places. We already reported it as being empty
everywhere else, except inside of <references>.

Note that the test cases look like they are reporting the same errors
twice. But this is not the case:

The first set of errors is about <ref name="…"> inside of <references>
not having visible content. This should always be reported, even if the
<ref> got content from somewhere else on the page.

The second set of errors is when a <ref name="…"> *never* got any
content.

This patch will slightly increase the numbers of errors reported.

Change-Id: I4a156aa9e466f735d92fe0ba5cc0678ec8bbdd50
2019-12-17 13:37:01 +01:00
Thiemo Kreuz 51ff3cc819 Several code cleanups after getting rid of cloning
* Use the Html class to safely create HTML code.
* $this->referenceStack can not be null any more.
* $this->inReferencesGroup is not needed during output, only when
  parsing tags.
* Replace ReferencesStack::getGroupRefs() as well as deleteGroup()
  with a combined popGroup() that does both things.
* Extract the code responsible for the "responsive" behavior to a
  separate function.
* Some TestingAccessWrapper are not needed.

Change-Id: Ie1cf2533d7417ae2f6647664ff1145e37b814a39
2019-12-16 15:47:23 +01:00
Adam Wight 870b9ec181 Rename test to match class
Change-Id: I1814f31ead3882d4c484e0da2808fc7fbc9d2f37
2019-12-12 11:15:13 +01:00
Adam Wight ab07a3253c Remove Parser state from CiteErrorReporter
Finishes breaking the circular reference between Cite and Parser.

This patch also demonstrates how evil it is to allow the error reporter
to be called from anywhere, and have side-effects.  At least it's explicit
now.

Also fixes a bug where the inner error message would not be in the
interface language.

Bug: T240431
Change-Id: Ic3325cafb503e78295d72231ac6da5c121402def
2019-12-12 11:15:07 +01:00
Adam Wight 22b2f78db6 Remove Parser state from ReferencesFormatter
Bug: T240431
Change-Id: I3477a64b9a9dba0cfe890c4b598a51c2f971c76c
2019-12-12 11:12:17 +01:00
Adam Wight 2a5976f007 Remove Parser state from FootnoteMarkFormatter
Bug: T240431
Change-Id: Ie53444114c032e083293d3b5325252debb0640a7
2019-12-12 11:12:17 +01:00
Adam Wight 852a503262 Don't keep parser reference in Cite
This begins our journey of breaking the circular reference between
Cite and Parser.  In later patches the child objects will also take
Parser as a parameter.

Bug: T240431
Change-Id: Ic672bb4bae19ac5f1e1f5817de171d76b3bd8786
2019-12-12 11:12:17 +01:00
Adam Wight a227395e3a Lazy instantiation of Cite
Only create a Cite object if we need one.  Never clearState, just
destroy and recreate later.

This makes it less likely that we leak state between parsers, and
saves memory and processing on pages without references.

It's also preparation to decouple Cite logic from state.

Change-Id: I3db517591f4131c23151c76c223af7419cc00ae9
2019-12-12 11:12:17 +01:00
Thiemo Kreuz 3f2aeb7e31 Rename two Cite… classes and clean up test setups
* All classes are in a Cite\ namespace now. No need to repeat the word
"Cite" all over the place.

* The "key formatter" is more an ID or anchor formatter. The strings it
returns are all used in id="…" attributes, as well as in href="#…" links
to jump to these IDs.

* This patch also removes quite a bunch of callbacks from tests that
don't need to be callbacks.

* I'm also replacing all json_encode().

* To make the test code more readable, I shorten a bunch of variable
names to e.g. $msg. The fact they are mocks is still relevant, and still
visible because these variable names are only used in very short scopes.

Change-Id: I2bd7c731efd815bcdc5d33bccb0c8e280d55bd06
2019-12-12 08:48:02 +01:00
jenkins-bot 9cc9e9cdc1 Merge "Add parser tests for reused extended <ref> before defined" 2019-12-11 16:04:53 +00:00
jenkins-bot 5b7f6d2d35 Merge "Add parser test for duplicate extended references" 2019-12-11 15:35:33 +00:00
jenkins-bot 0dafe64305 Merge "Rename CiteParserTagHooks::initialize to register" 2019-12-11 15:35:32 +00:00
jenkins-bot 5d5fd30a3c Merge "Minor improvements to the test coverage" 2019-12-11 15:35:32 +00:00
Thiemo Kreuz f86b5073fd Add parser tests for reused extended <ref> before defined
Bug: T240424
Change-Id: I945c2e12cfa3ff851380a1ff4491c8af076f523a
2019-12-11 16:30:17 +01:00
Thiemo Kreuz 193b840010 Add parser test for duplicate extended references
Bug: T240459
Change-Id: Ifc7a695e89a49ccc6c66d49efe41b2321b0915f0
2019-12-11 15:58:34 +01:00
jenkins-bot e39b1d6cbd Merge "Use messagelocalizer in CiteErrorReporter" 2019-12-11 11:42:28 +00:00
Thiemo Kreuz d0cb639e03 Minor improvements to the test coverage
We are *so* close to 90%.

This patch should raise the coverage for the CiteDataModule to 100%.
I'm also adding a pure unit test for the clone() behavior. Note the
later is already covered by the CiteDbTest.

Question: Do we want the CiteDbTest to @cover anything?

Change-Id: I40763d01e18991f509bc30b6655aa57b23412fd9
2019-12-11 12:22:59 +01:00
Adam Wight f93f1b4fe0 Use messagelocalizer in CiteErrorReporter
Fixes a bug introduced in Icf61c9a27fd, which would cause a parser
cache split any time the Cite extension was initialized.  The
`setLanguage` interface is regrettable, but I'm hoping it will only
be around temporarily.

Converts an integration test into a unit test and completes coverage.

Bug: T239988
Change-Id: I4b1f8909700845c9fa0cbc1a3de50ee7d42f69a5
2019-12-11 09:53:47 +01:00
jenkins-bot 6556f75c38 Merge "Integration parser test for ParserFunctions" 2019-12-11 08:45:34 +00:00
jenkins-bot 76b4706938 Merge "Rename formatNumNoSeparators() to localizeDigits()" 2019-12-11 08:29:09 +00:00
Adam Wight 084ca6b3d4 Integration parser test for ParserFunctions
Depends-On: I09844079f163e583d3b1e941c701f8cda5029a0a
Bug: T240345
Change-Id: I86c55ff88d9f4b800e8868728dfec6b4ceda82c2
2019-12-11 08:58:58 +01:00