Skip to content

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Oct 24, 2025

Thanks to @mysteq for pointing out here.

@hvitved hvitved added no-change-note-required This PR does not need a change note and removed JS documentation labels Oct 24, 2025
@github-actions
Copy link
Contributor

QHelp previews:

javascript/ql/src/Expressions/ExprHasNoEffect.qhelp

Expression has no effect

An expression that has no effects (such as changing variable values or producing output) and occurs in a context where its value is ignored possibly indicates missing code or a latent bug.

Recommendation

Carefully inspect the expression to ensure it is not a symptom of a bug. To document that the value of an expression is deliberately ignored, wrap it into a void expression.

Example

The following code snippet accesses the selectedIndex property of a DOM node to trigger additional processing in certain versions of Safari. This, however, is not clear from the code itself, which looks like a property read whose value is discarded immediately.

elem.parentNode.selectedIndex;

To document the fact that the property read has a hidden side effect and its value is deliberately ignored, it should be wrapped into a void expression like this:

void(elem.parentNode.selectedIndex);

A common source of warnings are constructor functions that "declare" a property of the newly constructed object without initializing it, by simply referring to it in an expression statement like this:

function Graph(nodes, edges) {
  this.nodes = nodes;
  this.edges = edges;
  // cache minimum distance between pairs of nodes
  this.distance;
}

Semantically, this is unnecessary, since the property will be created upon first assignment. If the aim is to document the existence of the property, it would be better to explicitly assign it an initial value, which also serves to document its expected type:

function Graph(nodes, edges) {
  this.nodes = nodes;
  this.edges = edges;
  // cache minimum distance between pairs of nodes
  this.distance = {};
}
javascript/ql/src/LanguageFeatures/DeleteVar.qhelp

Deleting non-property

The delete operator should only be used to delete properties from objects. Using it to delete variables makes code hard to maintain and will break in strict mode.

Recommendation

If the variable you are deleting is a global variable, this is a sign that your code relies too much on global state. Try encapsulating this global state by means of one of the module patterns introduced in JavaScript: The Good Parts.

Example

In the following code snippet, delete is used to clean up the global cache variable used by function get.

var cache;

function init() {
	cache = {};
}

function done() {
	delete cache;
}

function get(k) {
	k = '$' + k;
	if (!cache.hasOwnProperty(k))
		cache[k] = compute(k);
	return cache[k];
}

function compute(k) {
	// compute value for k
	// ...
}

It would be clearer to wrap the whole module into a closure like this (which also avoids exposing function compute to the outside world):

(function(global) {
	var cache;

	global.init = function init() {
		cache = {};
	};

	global.done = function done() {
	};

	global.get = function get(k) {
		k = '$' + k;
		if (!cache.hasOwnProperty(k))
			cache[k] = compute(k);
		return cache[k];
	}

	function compute(k) {
		// compute value for k
		// ...
	}
}(this));

@hvitved hvitved marked this pull request as ready for review October 24, 2025 07:18
@hvitved hvitved requested a review from a team as a code owner October 24, 2025 07:18
Copilot AI review requested due to automatic review settings October 24, 2025 07:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes two invalid external reference links from QHelp documentation files that were pointing to the now-defunct jslinterrors.com website.

  • Removed broken JSLint Error Explanations references from two QHelp files
  • Cleaned up empty reference sections that only contained the invalid links

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
javascript/ql/src/LanguageFeatures/DeleteVar.qhelp Removed broken jslinterrors.com reference link and empty references section
javascript/ql/src/Expressions/ExprHasNoEffect.qhelp Removed broken jslinterrors.com reference link and empty references section

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@Napalys Napalys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@hvitved hvitved merged commit 74411ff into github:main Oct 24, 2025
12 checks passed
@hvitved hvitved deleted the js/remove-invalid-qhelp-links branch October 24, 2025 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants