jenkins-bot has submitted this change and it was merged.
Change subject: Replace abstract functions that just throw with null
......................................................................
Replace abstract functions that just throw with null
Per Timo, this avoids us having pointless code that is never
executed, and the replacement notification to the user (that
ve.blah.UnicornAnnotation.foo is null and not a method) is
sufficiently clear that the issue lies in the lack of existence
of the ve.blah.UnicornAnnotation#foo method.
This removes 46 such methods from our codebase (1970 -> 1924),
and so incidentally boosts nominal code coverage for the functions
from 60.61% to 62.06%. Progress.
Bug: T54482
Change-Id: I9480e7375cb3e3487fde19634b1f545131ee5dde
---
M src/ce/nodes/ve.ce.GeneratedContentNode.js
M src/dm/ve.dm.Annotation.js
M src/dm/ve.dm.ResizableNode.js
M src/dm/ve.dm.Selection.js
M src/init/ve.init.Platform.js
M src/ui/inspectors/ve.ui.AnnotationInspector.js
M src/ui/ve.ui.Context.js
M src/ui/ve.ui.DataTransferHandler.js
M src/ui/ve.ui.Surface.js
M src/ve.Node.js
10 files changed, 80 insertions(+), 153 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/ce/nodes/ve.ce.GeneratedContentNode.js
b/src/ce/nodes/ve.ce.GeneratedContentNode.js
index 8fd9719..b863324 100644
--- a/src/ce/nodes/ve.ce.GeneratedContentNode.js
+++ b/src/ce/nodes/ve.ce.GeneratedContentNode.js
@@ -67,12 +67,11 @@
* by forceUpdate().
*
* @abstract
+ * @method
* @param {Object} [config] Optional additional data
* @returns {jQuery.Promise} Promise object, may be abortable
*/
-ve.ce.GeneratedContentNode.prototype.generateContents = function () {
- throw new Error( 've.ce.GeneratedContentNode subclass must implement
generateContents' );
-};
+ve.ce.GeneratedContentNode.prototype.generateContents = null;
/* Methods */
diff --git a/src/dm/ve.dm.Annotation.js b/src/dm/ve.dm.Annotation.js
index b79b652..69694ec 100644
--- a/src/dm/ve.dm.Annotation.js
+++ b/src/dm/ve.dm.Annotation.js
@@ -80,9 +80,7 @@
* @param {Node[]} childDomElements Children that will be appended to the
returned element
* @returns {HTMLElement[]} Array of DOM elements; only the first element is
used; may be empty
*/
-ve.dm.Annotation.static.toDomElements = function () {
- throw new Error( 've.dm.Annotation subclass must implement
toDomElements' );
-};
+ve.dm.Annotation.static.toDomElements = null;
/**
* @inheritdoc
@@ -101,6 +99,7 @@
/**
* Convenience wrapper for .toDomElements() on the current annotation
+ *
* @method
* @param {HTMLDocument} [doc] HTML document to use to create elements
* @see ve.dm.Model#toDomElements
diff --git a/src/dm/ve.dm.ResizableNode.js b/src/dm/ve.dm.ResizableNode.js
index 9f5e09b..0a26ed5 100644
--- a/src/dm/ve.dm.ResizableNode.js
+++ b/src/dm/ve.dm.ResizableNode.js
@@ -41,11 +41,10 @@
* Create a scalable object based on the current object's width and height.
*
* @abstract
+ * @method
* @returns {ve.dm.Scalable} Scalable object
*/
-ve.dm.ResizableNode.prototype.createScalable = function () {
- throw new Error( 've.dm.ResizableNode subclass must implement
createScalable' );
-};
+ve.dm.ResizableNode.prototype.createScalable = null;
/**
* Handle attribute change events from the model.
diff --git a/src/dm/ve.dm.Selection.js b/src/dm/ve.dm.Selection.js
index 0227d95..ea52fc1 100644
--- a/src/dm/ve.dm.Selection.js
+++ b/src/dm/ve.dm.Selection.js
@@ -46,98 +46,98 @@
/**
* Create a new selection from a hash object
*
+ * @abstract
+ * @method
* @param {ve.dm.Document} doc Document to create the selection on
* @param {Object} hash Hash object
* @returns {ve.dm.Selection} New selection
*/
-ve.dm.Selection.static.newFromHash = function () {
- throw new Error( 've.dm.Selection subclass must implement newFromHash'
);
-};
+ve.dm.Selection.static.newFromHash = null;
/* Methods */
/**
* Get a JSON serialization of this selection
*
+ * @abstract
+ * @method
* @returns {Object} Object for JSON serialization
*/
-ve.dm.Selection.prototype.toJSON = function () {
- throw new Error( 've.dm.Selection subclass must implement toJSON' );
-};
+ve.dm.Selection.prototype.toJSON = null;
/**
* Get a textual description of this selection, for debugging purposes
*
+ * @abstract
+ * @method
* @returns {string} Textual description
*/
-ve.dm.Selection.prototype.getDescription = function () {
- throw new Error( 've.dm.Selection subclass must implement
getDescription' );
-};
+ve.dm.Selection.prototype.getDescription = null;
/**
* Create a copy of this selection
*
+ * @abstract
+ * @method
* @returns {ve.dm.Selection} Cloned selection
*/
-ve.dm.Selection.prototype.clone = function () {
- throw new Error( 've.dm.Selection subclass must implement clone' );
-};
+ve.dm.Selection.prototype.clone = null;
/**
* Get a new selection at the start point of this one
*
+ * @abstract
+ * @method
* @returns {ve.dm.Selection} Collapsed selection
*/
-ve.dm.Selection.prototype.collapseToStart = function () {
- throw new Error( 've.dm.Selection subclass must implement
collapseToStart' );
-};
+ve.dm.Selection.prototype.collapseToStart = null;
/**
* Get a new selection at the end point of this one
*
+ * @abstract
+ * @method
* @returns {ve.dm.Selection} Collapsed selection
*/
-ve.dm.Selection.prototype.collapseToEnd = function () {
- throw new Error( 've.dm.Selection subclass must implement
collapseToEnd' );
-};
+ve.dm.Selection.prototype.collapseToEnd = null;
/**
* Get a new selection at the 'from' point of this one
*
+ * @abstract
+ * @method
* @returns {ve.dm.Selection} Collapsed selection
*/
-ve.dm.Selection.prototype.collapseToFrom = function () {
- throw new Error( 've.dm.Selection subclass must implement
collapseToFrom' );
-};
+ve.dm.Selection.prototype.collapseToFrom = null;
/**
* Get a new selection at the 'to' point of this one
*
+ * @abstract
+ * @method
* @returns {ve.dm.Selection} Collapsed selection
*/
-ve.dm.Selection.prototype.collapseToTo = function () {
- throw new Error( 've.dm.Selection subclass must implement collapseToTo'
);
-};
+ve.dm.Selection.prototype.collapseToTo = null;
/**
* Check if a selection is collapsed
*
+ * @abstract
+ * @method
* @returns {boolean} Selection is collapsed
*/
-ve.dm.Selection.prototype.isCollapsed = function () {
- throw new Error( 've.dm.Selection subclass must implement isCollapsed'
);
-};
+ve.dm.Selection.prototype.isCollapsed = null;
/**
* Apply translations from a transaction
*
+ * @abstract
+ * @method
* @param {ve.dm.Transaction} tx Transaction
* @param {boolean} [excludeInsertion] Do not grow to cover insertions at
boundaries
* @return {ve.dm.Selection} A new translated selection
*/
-ve.dm.Selection.prototype.translateByTransaction = function () {
- throw new Error( 've.dm.Selection subclass must implement
translateByTransaction' );
-};
+ve.dm.Selection.prototype.translateByTransaction = null;
/**
* Apply translations from a set of transactions
@@ -166,11 +166,11 @@
/**
* Get the content ranges for this selection
*
+ * @abstract
+ * @method
* @returns {ve.Range[]} Ranges
*/
-ve.dm.Selection.prototype.getRanges = function () {
- throw new Error( 've.dm.Selection subclass must implement getRanges' );
-};
+ve.dm.Selection.prototype.getRanges = null;
/**
* Get the document model this selection applies to
@@ -184,12 +184,12 @@
/**
* Check if two selections are equal
*
+ * @abstract
+ * @method
* @param {ve.dm.Selection} other Other selection
* @returns {boolean} Selections are equal
*/
-ve.dm.Selection.prototype.equals = function () {
- throw new Error( 've.dm.Selection subclass must implement equals' );
-};
+ve.dm.Selection.prototype.equals = null;
/* Factory */
diff --git a/src/init/ve.init.Platform.js b/src/init/ve.init.Platform.js
index d0d561f..12a1340 100644
--- a/src/init/ve.init.Platform.js
+++ b/src/init/ve.init.Platform.js
@@ -66,9 +66,7 @@
* @abstract
* @returns {RegExp} Regular expression object
*/
-ve.init.Platform.prototype.getExternalLinkUrlProtocolsRegExp = function () {
- throw new Error( 've.init.Platform.getExternalLinkUrlProtocolsRegExp
must be overridden in subclass' );
-};
+ve.init.Platform.prototype.getExternalLinkUrlProtocolsRegExp = null;
/**
* Get a config value from the platform.
@@ -78,9 +76,7 @@
* @param {string|string[]} key Config key, or list of keys
* @returns {Mixed|Object} Config value, or keyed object of config values if
list of keys provided
*/
-ve.init.Platform.prototype.getConfig = function () {
- throw new Error( 've.init.Platform.getConfig must be overridden in
subclass' );
-};
+ve.init.Platform.prototype.getConfig = null;
/**
* Add multiple messages to the localization system.
@@ -89,9 +85,7 @@
* @abstract
* @param {Object} messages Containing plain message values
*/
-ve.init.Platform.prototype.addMessages = function () {
- throw new Error( 've.init.Platform.addMessages must be overridden in
subclass' );
-};
+ve.init.Platform.prototype.addMessages = null;
/**
* Get a message from the localization system.
@@ -102,9 +96,7 @@
* @param {Mixed...} [args] List of arguments which will be injected at $1,
$2, etc. in the message
* @returns {string} Localized message, or key or '<' + key + '>' if message
not found
*/
-ve.init.Platform.prototype.getMessage = function () {
- throw new Error( 've.init.Platform.getMessage must be overridden in
subclass' );
-};
+ve.init.Platform.prototype.getMessage = null;
/**
* Add multiple parsed messages to the localization system.
@@ -113,9 +105,7 @@
* @abstract
* @param {Object} messages Map of message-key/html pairs
*/
-ve.init.Platform.prototype.addParsedMessages = function () {
- throw new Error( 've.init.Platform.addParsedMessages must be overridden
in subclass' );
-};
+ve.init.Platform.prototype.addParsedMessages = null;
/**
* Get a parsed message as HTML string.
@@ -127,9 +117,7 @@
* @param {string} key Message key
* @returns {string} Parsed localized message as HTML string
*/
-ve.init.Platform.prototype.getParsedMessage = function () {
- throw new Error( 've.init.Platform.getParsedMessage must be overridden
in subclass' );
-};
+ve.init.Platform.prototype.getParsedMessage = null;
/**
* Get the user language and any fallback languages.
@@ -138,9 +126,7 @@
* @abstract
* @returns {string[]} User language strings
*/
-ve.init.Platform.prototype.getUserLanguages = function () {
- throw new Error( 've.init.Platform.getUserLanguages must be overridden
in subclass' );
-};
+ve.init.Platform.prototype.getUserLanguages = null;
/**
* Get a list of URL entry points where media can be found.
@@ -149,9 +135,7 @@
* @abstract
* @returns {string[]} API URLs
*/
-ve.init.Platform.prototype.getMediaSources = function () {
- throw new Error( 've.init.Platform.getMediaSources must be overridden
in subclass' );
-};
+ve.init.Platform.prototype.getMediaSources = null;
/**
* Get a list of all language codes.
@@ -160,9 +144,7 @@
* @abstract
* @returns {string[]} Language codes
*/
-ve.init.Platform.prototype.getLanguageCodes = function () {
- throw new Error( 've.init.Platform.getLanguageCodes must be overridden
in subclass' );
-};
+ve.init.Platform.prototype.getLanguageCodes = null;
/**
* Get a language's name from its code, in the current user language if
possible.
@@ -172,9 +154,7 @@
* @param {string} code Language code
* @returns {string} Language name
*/
-ve.init.Platform.prototype.getLanguageName = function () {
- throw new Error( 've.init.Platform.getLanguageName must be overridden
in subclass' );
-};
+ve.init.Platform.prototype.getLanguageName = null;
/**
* Get a language's autonym from its code.
@@ -184,9 +164,7 @@
* @param {string} code Language code
* @returns {string} Language autonym
*/
-ve.init.Platform.prototype.getLanguageAutonym = function () {
- throw new Error( 've.init.Platform.getLanguageAutonym must be
overridden in subclass' );
-};
+ve.init.Platform.prototype.getLanguageAutonym = null;
/**
* Get a language's direction from its code.
@@ -196,9 +174,7 @@
* @param {string} code Language code
* @returns {string} Language direction
*/
-ve.init.Platform.prototype.getLanguageDirection = function () {
- throw new Error( 've.init.Platform.getLanguageDirection must be
overridden in subclass' );
-};
+ve.init.Platform.prototype.getLanguageDirection = null;
/**
* Initialize the platform. The default implementation is to do nothing and
return a resolved
diff --git a/src/ui/inspectors/ve.ui.AnnotationInspector.js
b/src/ui/inspectors/ve.ui.AnnotationInspector.js
index c35a0bd..e0954a6 100644
--- a/src/ui/inspectors/ve.ui.AnnotationInspector.js
+++ b/src/ui/inspectors/ve.ui.AnnotationInspector.js
@@ -88,28 +88,20 @@
* but existing annotations won't be removed either.
*
* @abstract
+ * @method
* @returns {ve.dm.Annotation} Annotation to apply
- * @throws {Error} If not overridden in subclass
*/
-ve.ui.AnnotationInspector.prototype.getAnnotation = function () {
- throw new Error(
- 've.ui.AnnotationInspector.getAnnotation not implemented in
subclass'
- );
-};
+ve.ui.AnnotationInspector.prototype.getAnnotation = null;
/**
* Get an annotation object from a fragment.
*
* @abstract
+ * @method
* @param {ve.dm.SurfaceFragment} fragment Surface fragment
* @returns {ve.dm.Annotation} Annotation
- * @throws {Error} If not overridden in a subclass
*/
-ve.ui.AnnotationInspector.prototype.getAnnotationFromFragment = function () {
- throw new Error(
- 've.ui.AnnotationInspector.getAnnotationFromFragment not
implemented in subclass'
- );
-};
+ve.ui.AnnotationInspector.prototype.getAnnotationFromFragment = null;
/**
* Get matching annotations within a fragment.
diff --git a/src/ui/ve.ui.Context.js b/src/ui/ve.ui.Context.js
index 89b356a..dab3a85 100644
--- a/src/ui/ve.ui.Context.js
+++ b/src/ui/ve.ui.Context.js
@@ -322,11 +322,8 @@
* @method
* @abstract
* @return {ve.ui.WindowManager} Inspector window manager
- * @throws {Error} If this method is not overridden in a concrete subclass
*/
-ve.ui.Context.prototype.createInspectorWindowManager = function () {
- throw new Error( 've.ui.Context.createInspectorWindowManager must be
overridden in subclass' );
-};
+ve.ui.Context.prototype.createInspectorWindowManager = null;
/**
* Toggle the menu.
diff --git a/src/ui/ve.ui.DataTransferHandler.js
b/src/ui/ve.ui.DataTransferHandler.js
index 9bdc520..3ef53d3 100644
--- a/src/ui/ve.ui.DataTransferHandler.js
+++ b/src/ui/ve.ui.DataTransferHandler.js
@@ -84,10 +84,11 @@
* Process the file
*
* Implementations should aim to resolve this.insertableDataDeferred.
+ *
+ * @abstract
+ * @method
*/
-ve.ui.DataTransferHandler.prototype.process = function () {
- throw new Error( 've.ui.DataTransferHandler subclass must implement
process' );
-};
+ve.ui.DataTransferHandler.prototype.process = null;
/**
* Insert the file at a specified fragment
diff --git a/src/ui/ve.ui.Surface.js b/src/ui/ve.ui.Surface.js
index 9723a58..dbfe38e 100644
--- a/src/ui/ve.ui.Surface.js
+++ b/src/ui/ve.ui.Surface.js
@@ -167,11 +167,8 @@
* @method
* @abstract
* @return {ve.ui.Context} Context
- * @throws {Error} If this method is not overridden in a concrete subclass
*/
-ve.ui.Surface.prototype.createContext = function () {
- throw new Error( 've.ui.Surface.createContext must be overridden in
subclass' );
-};
+ve.ui.Surface.prototype.createContext = null;
/**
* Create a dialog window manager.
@@ -179,11 +176,8 @@
* @method
* @abstract
* @return {ve.ui.WindowManager} Dialog window manager
- * @throws {Error} If this method is not overridden in a concrete subclass
*/
-ve.ui.Surface.prototype.createDialogWindowManager = function () {
- throw new Error( 've.ui.Surface.createDialogWindowManager must be
overridden in subclass' );
-};
+ve.ui.Surface.prototype.createDialogWindowManager = null;
/**
* Set up the debug bar and insert it into the DOM.
diff --git a/src/ve.Node.js b/src/ve.Node.js
index 523e9dc..8cf4722 100644
--- a/src/ve.Node.js
+++ b/src/ve.Node.js
@@ -47,9 +47,7 @@
* @abstract
* @returns {string[]|null} List of node types allowed as children or null if
any type is allowed
*/
-ve.Node.prototype.getChildNodeTypes = function () {
- throw new Error( 've.Node.getChildNodeTypes must be overridden in
subclass' );
-};
+ve.Node.prototype.getChildNodeTypes = null;
/**
* Get allowed parent node types.
@@ -58,9 +56,7 @@
* @abstract
* @returns {string[]|null} List of node types allowed as parents or null if
any type is allowed
*/
-ve.Node.prototype.getParentNodeTypes = function () {
- throw new Error( 've.Node.getParentNodeTypes must be overridden in
subclass' );
-};
+ve.Node.prototype.getParentNodeTypes = null;
/**
* Check if the specified type is an allowed child node type
@@ -91,9 +87,7 @@
* @abstract
* @returns {string[]|null} List of node types suggested as parents or null if
any type is suggested
*/
-ve.Node.prototype.getSuggestedParentNodeTypes = function () {
- throw new Error( 've.Node.getSuggestedParentNodeTypes must be
overridden in subclass' );
-};
+ve.Node.prototype.getSuggestedParentNodeTypes = null;
/**
* Check if the node can have children.
@@ -102,9 +96,7 @@
* @abstract
* @returns {boolean} Node can have children
*/
-ve.Node.prototype.canHaveChildren = function () {
- throw new Error( 've.Node.canHaveChildren must be overridden in
subclass' );
-};
+ve.Node.prototype.canHaveChildren = null;
/**
* Check if the node can have children but not content nor be content.
@@ -113,9 +105,7 @@
* @abstract
* @returns {boolean} Node can have children but not content nor be content
*/
-ve.Node.prototype.canHaveChildrenNotContent = function () {
- throw new Error( 've.Node.canHaveChildrenNotContent must be overridden
in subclass' );
-};
+ve.Node.prototype.canHaveChildrenNotContent = null;
/**
* Check if the node can contain content.
@@ -124,9 +114,7 @@
* @abstract
* @returns {boolean} Node can contain content
*/
-ve.Node.prototype.canContainContent = function () {
- throw new Error( 've.Node.canContainContent must be overridden in
subclass' );
-};
+ve.Node.prototype.canContainContent = null;
/**
* Check if the node is content.
@@ -135,9 +123,7 @@
* @abstract
* @returns {boolean} Node is content
*/
-ve.Node.prototype.isContent = function () {
- throw new Error( 've.Node.isContent must be overridden in subclass' );
-};
+ve.Node.prototype.isContent = null;
/**
* Check if the node has a wrapped element in the document data.
@@ -146,9 +132,7 @@
* @abstract
* @returns {boolean} Node represents a wrapped element
*/
-ve.Node.prototype.isWrapped = function () {
- throw new Error( 've.Node.isWrapped must be overridden in subclass' );
-};
+ve.Node.prototype.isWrapped = null;
/**
* Check if the node is focusable
@@ -157,9 +141,7 @@
* @abstract
* @returns {boolean} Node is focusable
*/
-ve.Node.prototype.isFocusable = function () {
- throw new Error( 've.Node.isFocusable must be overridden in subclass' );
-};
+ve.Node.prototype.isFocusable = null;
/**
* Check if the node is alignable
@@ -168,9 +150,7 @@
* @abstract
* @returns {boolean} Node is alignable
*/
-ve.Node.prototype.isAlignable = function () {
- throw new Error( 've.Node.isAlignable must be overridden in subclass' );
-};
+ve.Node.prototype.isAlignable = null;
/**
* Check if the node has significant whitespace.
@@ -181,9 +161,7 @@
* @abstract
* @returns {boolean} Node has significant whitespace
*/
-ve.Node.prototype.hasSignificantWhitespace = function () {
- throw new Error( 've.Node.hasSignificantWhitespace must be overridden
in subclass' );
-};
+ve.Node.prototype.hasSignificantWhitespace = null;
/**
* Check if the node handles its own children
@@ -192,9 +170,7 @@
* @abstract
* @returns {boolean} Node handles its own children
*/
-ve.Node.prototype.handlesOwnChildren = function () {
- throw new Error( 've.Node.handlesOwnChildren must be overridden in
subclass' );
-};
+ve.Node.prototype.handlesOwnChildren = null;
/**
* Check if the node's children should be ignored.
@@ -203,9 +179,7 @@
* @abstract
* @returns {boolean} Node's children should be ignored
*/
-ve.Node.prototype.shouldIgnoreChildren = function () {
- throw new Error( 've.Node.ignoreChildren must be overridden in
subclass' );
-};
+ve.Node.prototype.shouldIgnoreChildren = null;
/**
* Get the length of the node.
@@ -214,9 +188,7 @@
* @abstract
* @returns {number} Node length
*/
-ve.Node.prototype.getLength = function () {
- throw new Error( 've.Node.getLength must be overridden in subclass' );
-};
+ve.Node.prototype.getLength = null;
/**
* Get the offset of the node within the document.
@@ -228,9 +200,7 @@
* @returns {number} Offset of node
* @throws {Error} Node not found in parent's children array
*/
-ve.Node.prototype.getOffset = function () {
- throw new Error( 've.Node.getOffset must be overridden in subclass' );
-};
+ve.Node.prototype.getOffset = null;
/**
* Get the range inside the node.
--
To view, visit https://gerrit.wikimedia.org/r/203046
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9480e7375cb3e3487fde19634b1f545131ee5dde
Gerrit-PatchSet: 5
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Jforrester <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
