Commit graph

155 commits

Author SHA1 Message Date
thiemowmde 8f81812242 Enable and enforce a few extra PHPCS sniffs
This is not really anything new. Most code already followed these
sniffs. This patch just fixes the remaining exceptions. Also:
* Remove PHPDoc blocks that don't add anything but just repeat the
  strict types.
* Remove @file comments in favor of class-level comments.
* Add strict types where possible, most notably some `void`.

Change-Id: Iff6872dff68170b0fc4e82ac2ba3cad385e8773e
2023-09-06 18:15:22 +02:00
jenkins-bot bde2188280 Merge "Show much more actionable context when paramOrder is incomplete" 2023-09-04 13:45:24 +00:00
jenkins-bot 9f280126c0 Merge "Simplify test data providers with yield" 2023-08-23 22:30:21 +00:00
thiemowmde 416c3d4f45 Make use of the ??= and ?: operators where it makes sense
This patch also removes small pieces of code from tests that are
never called, as it turns out.

Change-Id: I10ce18339135fefd9a99e0613dc6bfc457ddf4df
2023-08-23 09:24:50 +02:00
thiemowmde b522972cac Simplify test data providers with yield
Change-Id: I6315ad66bcfa9437e1473d59776404c280b8ce4f
2023-08-23 09:20:25 +02:00
thiemowmde bb7b801b64 Show much more actionable context when paramOrder is incomplete
So far we show nothing but the index in the "paramOrder" array. This
is especially useless when a parameter is missing. The index just
points to the end of the array then.

Same when an unknown parameter appears. What the user needs is not
the index but the name of the unknown parameter.

I played around with a few formats. As suggested in this patch:
 Required property "paramOrder[ "foo" ]" not found.
 Invalid value for property "paramOrder[ "foo" ]".

A possible alternative is:
 Required property "paramOrder[0] ("foo")" not found.
 Invalid value for property "paramOrder[0] ("foo")".

Bug: T340377
Change-Id: I1dbef1b6e585d5b972a0c9a373a040aee6027cf3
2023-08-21 12:52:42 +02:00
gerritbot cfd2e5b664 Replace some moved Title class uses, now MediaWiki\Title\Title
Bug: T321681
Change-Id: I2a7aaadc47dd068ccea8fd8110054433973df52c
2023-08-19 18:11:41 +00:00
Ed Sanders 6107710ef1 Left align caption and put [Edit template data] inline
Bug: T339806
Change-Id: I69a46e64810edf458b8a8ee3e043f6dbc287c8aa
2023-08-05 15:13:22 +01:00
thiemowmde 5f4f15cb85 Stop using private properties in unit tests
Change-Id: I199d44882561c9b0ee84cab339a8e94dd03bf353
2023-06-28 08:16:34 +00:00
thiemowmde afd86e2970 Add missing strict types to all test code
Change-Id: I6dacbdb8c961e504694a9f3c45d551527eb1b448
2023-06-26 18:08:42 +02:00
thiemowmde b8f4df67f9 Replace inappropriate use of "word-separator" message
These aren't sentences but lists of aliases as well as suggested
values. They are rendered as <code>, visualized as light gray boxes.
We don't need them to respect some language-specific word separation
to be able to read them like a sentence.

https://translatewiki.net/wiki/MediaWiki:Word-separator/qqq
explains how the message should be used.

Bug: T340378
Change-Id: Ia082b5ccb211524262f0c535bb178ee7ec3c20eb
2023-06-25 16:57:50 +02:00
Jon Robson f45b0625ad Temporary disable parser test
We need to disable this so that we can update the message box in core
to use codex components.

Once I16d8f53b9a83a468acdb3803044f6c4e9d5dfeeb has been merged
the test will be restored using the new message box markup.

Bug: T326587
Change-Id: I3959c8f8d4c7bbe019f5e3b356649b5b7dc88907
2023-06-16 21:28:55 +00:00
Thiemo Kreuz 647cd0bc65 Clean up base class reaching into the subclass
This does not change anything. It's only about the readabilility
of the code.

Change-Id: Ia908bd90ebc261f3bebc7854501a6f5d5e736384
2023-06-09 07:10:19 +00:00
Umherirrender fe40e527c0 tests: Make PHPUnit data providers static
Initally used a new sniff with autofix (T333745)

Bug: T332865
Change-Id: Id3e2d5645dfd89d2b7d3586a4e1446d16a9ca60e
2023-05-20 14:10:49 +02:00
thiemowmde b632162245 Add missing validation for empty parameter names
Note this will make the templatedata API temporarily fail for
templates that are affected by this, until their documentation is
fixed. I think we have two ways to proceed here:

1. Thanks to Ie572809 we can change the API module to not do the
validation again. This allows us to make the validation more strict,
but the API module will continue to deliver the same data as before
until the parser cache is invalidated.

2. We accept it and merge this patch as it is. The problem is
extremely rare and easy to fix.

Bug: T333826
Change-Id: I16c7cc2328c47dde196e2dc07edb2eace33a624f
2023-04-11 19:08:37 +00:00
jenkins-bot 0298a2d116 Merge "Split validation and normalization into separate services" 2023-04-11 04:51:02 +00:00
thiemowmde 584fdcddf6 Extract serialization methods into TemplateDataStatus class
This makes the large Hook class quite a bit smaller.

Change-Id: I55229116eb16ccd9be21d1f34de5e52826ece2bf
2023-03-09 11:46:49 +01:00
thiemowmde da63a3fdc6 Add missing parser tests to cover parser hook and renderer
Without this the entire code in Hooks::render() is uncovered.

Change-Id: Id347b278b6b5eed5f660c317ed21410de7910a1b
2023-03-09 11:46:49 +01:00
Thiemo Kreuz 7a32cba3ef Split validation and normalization into separate services
This makes it possible to use these steps independent from each
other. For example, a future patch can get rid of the re-validation
that's done over and over again when the API is called.

A significant change is that this gets rid of an expensive deep
clone. It was necessary before exactly because validation and
normalization was intertwined. Normalized properties would mess with
the later inheritance.

Strictly splitting validation and normalization (and executing them
in this order) solved this. The only downside of this is that
inherited properties are validated twice. But this is much less of a
problem, compared to the deep clone, I would like to argue.

This was always covered by tests. You can still see the tests fail
when you flip the execution order of inheritance and parameter
validation.

Bug: T301337
Change-Id: Ie5728094f9ed813f53b709d8b5283c4b99dc7d63
2023-03-06 13:05:51 +00:00
Thiemo Kreuz 505a835c49 Extract named isValidCustomFormatString() method
Bug: T301337
Change-Id: I02e81c264e7ebeb60483d7eacf9fa7b27ad1e545
2022-12-15 12:15:35 +00:00
Ed Sanders 678d95251b Add an "Edit template data" button to the TemplateData output
This button, similar to an edit-section link, will launch the editor
and immediately open the TemplateDataGenerator UI.

Bug: T316759
Depends-On: Idb5e3c51a22361e0d9916d3c31444daeff310ed2
Change-Id: Ieb575c499c16d87c28972a55662ef0bd9cb72c06
2022-11-08 13:36:38 +00:00
Thiemo Kreuz 8497d85a2d Fix CSS styling of the HTML rendering broken since 2016
Some time ago there was a little bit of custom CSS applied to the HTML
table rendering. This is broken since patch I74214ea from 2016. This
patch renamed all CSS classes but forgot to update the PHP code
accordingly.

I decided to not change the HTML rendering because these class names
might already be used in custom per-wiki or per-user CSS. Instead I
partly revert I74214ea.

Unfortunately, some of the styles are quite dramatic, don't look good
or just don't work. I decided to remove some. The argument is that
the HTML rendering looks the same for 6 years now. I don't see a good
reason to change it now.

In detail:
* Suggested values are not aliases and should not be rendered in
  gray.
* The message "no description" is rendered in gray and italics. But
  this was applied to the wrong DOM element and made everything else
  gray and italic as well.
* The color #777 is not readable, violating WCAG rules. While it's ok
  to dim aliases and such, it must be at least #555 or darker.
* The "nowrap" destroys the table the moment one of the parameters
  does have a longer name or alias. Let the browser handle this, as
  it did for 6 years now.
* Same for rendering aliases as individual, indented blocks. This
  makes the table unnecessarily big when there are many aliases, and
  just doesn't look right. Again, let's stick to what we had for
  6 years.

Change-Id: Idfa76eed6e2d68474c79d4674efce091cb031b66
2022-08-08 11:56:16 +02:00
Thiemo Kreuz e297e767f0 Streamline HTML rendering code for format messages
The idea is to make it a little easier to follow what's going on
here.

This also improves an error message when tests fail.

Change-Id: If35be8aefab5a1568d53a9ecdc4313a66f71317b
2022-05-16 18:04:27 +00:00
Ed Sanders ef6294074e build: Update devDependencies
Change-Id: I80bd6849b1616b9c94e75eddfbf5f476b799e07b
2022-03-13 17:17:03 +00:00
Thiemo Kreuz d3f00177ba Don't extract template parameters from <pre>
<pre> behaves very similar to <nowiki> in so far that whatever it
contains doesn't get parsed as wikitext. It might contain {{{…}}}
tripple brackets, but these aren't going to be activated as template
parameters.

Bug: T91326
Change-Id: I05c24e369d97c48161c565e2ef30969ec28c6a23
2022-03-03 21:20:40 +00:00
jenkins-bot 67d6b77a84 Merge "Move getRawParams helper method to ApiTemplateData class" 2022-02-10 05:44:14 +00:00
Thiemo Kreuz da8969812c Move getRawParams helper method to ApiTemplateData class
This method is not used anywhere else:
https://codesearch.wmcloud.org/search/?q=getRawParams

I tried to make the code a bit more readable. Notable:
* Make use of the return value we get from the preg_… function.
* {{3,} means "the character '{' 3 or more times". {{{+ does the
  same. Note the { doesn't need to be escaped when it's not
  followed by a number.
* At the end, it doesn't make any difference when we scan for
  optional closing brackets. The moment we find at least 3 we are
  done.

The test is intentionally not moved. This is something for a later
patch.

Bug: T301337
Change-Id: I55e31ceecea2ae7c35bcfbc2d641b35f751820db
2022-02-09 12:37:33 +00:00
Thiemo Kreuz b1a4a27531 Split and streamline HTML formatter code
Bug: T301337
Change-Id: Ic2dac65e6e54411abc33326abf8d0fab148cb784
2022-02-09 12:37:29 +00:00
jenkins-bot 5407262418 Merge "Improve test coverage for HTML formatter code" 2022-02-08 16:09:17 +00:00
jenkins-bot a64ed0a376 Merge "Fix/narrow visibility of several methods" 2022-02-08 15:54:22 +00:00
Thiemo Kreuz a81a8b52bc Improve test coverage for HTML formatter code
… as well as some smaller improvements to the test coverage of other
code.

Change-Id: I5bec9c456fdc597c340dc0b515d23d837a7c5651
2022-02-03 12:45:27 +01:00
Thiemo Kreuz b674294789 Fix/narrow visibility of several methods
One method is only public to be able to test it. Others look like
they have been made "protected by default", which is not needed
anywhere.

Change-Id: Ib2231f0b2a879323aa53f8d40a175527c5b131d8
2022-02-03 10:48:04 +01:00
Thiemo Kreuz 001494f443 Move last remaining HTML formating code out of blob class
Effectively a no-op. This patch doesn't change what the code does.
Tests are in place to prove this.

As before, the tests are intentionally not moved but left in place.
This is for later patches to clean up.

Change-Id: If130e0d006a36d8c755288f8a4e4e9a4c42a6295
2022-02-03 09:33:03 +01:00
Thiemo Kreuz 8c24751491 Split validation and HTML formatting into separate services
No functional change was made to the code. It was only moved from one
place to another. Note there are a lot of tests that cover this code.
The tests haven't been touched on purpose. Splitting these as well
is something for a later patch.

Bug: T260980
Change-Id: I9fa0fa87768f2560b83a1b5f3d39211ea9d6cfad
2022-02-01 15:47:52 +01:00
Thiemo Kreuz 5f749c6418 Allow aliases to be integers in addition to strings
Parameter names in a template can be numeric. While it makes a lot of
sense to force a specific format in the TemplateData JSON (i.e. only
strings), it's inconvenient and confusing if numbers are rejected for
being "invalid".

Effects of this patch:
* The incoming JSON is allowed to contain numbers in the aliases
  array.
* However, the API normalizes these and forces all aliases to be
  strings, as it was always documented.
* The editor component accepts anything in the aliases array, but
  forces all aliases to be strings. Again, as documented.
* Note that it was never possible to use numeric keys in the `params`
  list. This patch is only about aliases.

At the moment this is a somewhat "hidden" feature. We might or might
not update the documentation to officially allow numeric aliases.

Bug: T298795
Change-Id: I32ea296b4520e7f21b03a1f6390db4f43b613bdd
2022-01-10 13:33:27 +01:00
Reedy 166812da07 Namespace extension
Change-Id: I779e97e512ec0c4f74fb6a4b706772fb1428e40f
2021-11-25 22:53:34 +00:00
Alexander Vorwerk df34af35af MediaWikiTestCase -> MediaWikiIntegrationTestCase
MediaWikiTestCase has been renamed to MediaWikiIntegrationTestCase in 1.34.

Bug: T293043
Change-Id: Ie93e91911784d519dc7017f4f2a3ba6310339389
2021-10-12 01:06:08 +02:00
jenkins-bot da2077d5bc Merge "Fix parameter auto-detection picking up syntax elements" 2021-10-06 23:27:08 +00:00
jenkins-bot 1c67ed5f97 Merge "Remove static keyword from all test code" 2021-10-06 23:27:06 +00:00
jenkins-bot f16838ebc0 Merge "Remove small pieces of unused code" 2021-10-06 23:05:34 +00:00
jenkins-bot 5d3820783a Merge "Use more generic @covers tags in Serialization test" 2021-10-04 08:34:44 +00:00
Thiemo Kreuz 2a9b7be921 Remove static keyword from all test code
I'm not sure why it was done this way. It doesn't need to.

Change-Id: Ie33ead5a3b6bddc464dd47e7e3153d6b8269b4c1
2021-10-02 10:43:07 +02:00
jenkins-bot 75ca493b68 Merge "Add test cases for (almost) all possible parsing errors" 2021-09-20 13:11:09 +00:00
Thiemo Kreuz d253fa4e28 Merge code paths in assertTemplateData() helper method
This streamlines the code of the helper method a bit, mostly by
avoiding duplication.

What actually happens is the exact same as before, with one
exception: When a test case doesn't have an expected "output",
the default (mostly empty) output does not run through the
roundtrip test. While doing this is not wrong, it doesn't tell
us anything about the specific test case.

Change-Id: I4a3d8a22c3dd6a9c5c3766195e5aef3cf37a6441
2021-09-06 08:48:01 +02:00
jenkins-bot 34503ed210 Merge "Use more strict assertSame() when comparing strings" 2021-09-04 05:19:49 +00:00
Thiemo Kreuz 3060559d1d Fix parameter auto-detection picking up syntax elements
See T290322 for a detailed description.

Bug: T290322
Change-Id: Id9935482fb466e7a1f6e55f042b13fe5851412d0
2021-09-03 13:18:42 +02:00
Thiemo Kreuz a7e1d60c64 Revert some unnecessary en→qqx changes
This reverts parts of I6c6342c.

Change-Id: I6e0e60604e393ffde21c3528ff3a26bfc01e66a2
2021-08-31 15:26:52 +02:00
jenkins-bot 324502fd88 Merge "Make all tests use dummy language qqx instead of en" 2021-08-30 17:14:04 +00:00
jenkins-bot 2b1cdff0af Merge "Fix several type hints in JavaScript code" 2021-08-30 17:05:56 +00:00
Thiemo Kreuz eb12e48b14 Add test cases for (almost) all possible parsing errors
"Almost" because I found at least one that appears to be
unreachable (the very first check for null). But changing this
code is out of scope of this patch.

This also updates some of the error messages to explain the
location of the error better. It appears like the incomplete
paths are copy-paste mistakes.

I also found one duplicate test case and removed it.

Change-Id: Ic0ee9d04f5cd1060ade385ef308e70d221dd2f18
2021-08-30 15:17:21 +00:00