Commit graph

3 commits

Author SHA1 Message Date
Bartosz Dziewoński af68c835bb Update exception handling for new code conventions
Change code to match the documented consensus formed on T321683:
https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP#Exception_handling

* Do not directly throw Exception, Error or MWException
* Document checked exceptions with @throws
* Do not document unchecked exceptions

For this extension, I think it makes sense to consider DOMException an
unchecked exception too (in addition to the usual LogicException and
RuntimeException).

Depends-On: Id07e301c3f20afa135e5469ee234a27354485652
Depends-On: I869af06896b9757af18488b916211c5a41a8c563
Depends-On: I42d9b7465d1406a22ef1b3f6d8de426c60c90e2c
Change-Id: Ic9d9efd031a87fa5a93143f714f0adb20f0dd956
2023-01-22 18:17:11 +00:00
C. Scott Ananian 25272e7a4a Don't refer directly to PHP dom extension classes; avoid nonstandard behavior
These changes ensure that DiscussionTools is independent of DOM
library choice, and will not break if/when Parsoid switches to an
alternate (more standards-compliant) DOM library.

We run `phan` against the Dodo standards-compliant DOM library,
so this ends up flagging uses of non-standard PHP extensions to
the DOM.  These will be suppressed for now with a "Nonstandard DOM"
comment that can be grepped for, since they will eventually
will need to be rewritten or worked around.

Most frequent issues:

* Node::nodeValue and Node::textContent and Element::getAttribute()
can return null in a spec-compliant implementation.  Add `?? ''` to
make spec-compliant results consistent w/ what PHP returns.

* DOMXPath doesn't accept anything except DOMDocument.  These uses
should be replaced with DOMCompat::querySelectorAll() or similar
(which end up using DOMXPath under the covers for DOMDocument any way,
but are implemented more efficiently in a spec-compliant
implementation).

* A couple of times we have code like:
  `while ($node->firstChild!==null) { $node = $node->firstChild; }`
and phan's analysis isn't strong enough to determine that $node is still
non-null after the while.  This same issue should appear with DOMDocument
but phan doesn't complain for some reason.

One apparently legit issue:

* Node::insertBefore() is once called in a funny way which leans on
the fact that the second option is optional in PHP.  This seems to be
a workaround for an ancient PHP bug, and can probably be safely
removed.

Bug: T287611
Bug: T217867
Change-Id: I3c4f41c3819770f85d68157c9f690d650b7266a3
2021-07-30 18:15:40 -04:00
Bartosz Dziewoński 308c2747b0 CommentParser.php: Use tree walking instead of XPath
This is similar to what the JS version does.

The TreeWalker and NodeFilter classes are adapted from
https://github.com/Krinkle/dom-TreeWalker-polyfill
(MIT license).

This makes #getComments twice as fast on en-big-oldparser.html

Change-Id: I2441f33e6e7bad753ac830d277e6a2e81ee8c93d
2020-07-15 16:40:50 +00:00