Skip to content

Conversation

aibaars
Copy link
Contributor

@aibaars aibaars commented Sep 5, 2025

I spotted a few non-breaking spaces in our published query help that seemed to serve no purpose. This PR drop uses of such spaces from our code.

The only ones left are in:

$ git grep -I -l ' '                                                                                                                                            
csharp/ql/integration-tests/all-platforms/blazor/BlazorTest/wwwroot/bootstrap/bootstrap.min.css
csharp/ql/integration-tests/all-platforms/blazor/BlazorTest/wwwroot/bootstrap/bootstrap.min.css.map
csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/BlazorTest/wwwroot/bootstrap/bootstrap.min.css
csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/BlazorTest/wwwroot/bootstrap/bootstrap.min.css.map
csharp/ql/integration-tests/all-platforms/blazor_net_8/BlazorTest/wwwroot/bootstrap/bootstrap.min.css
csharp/ql/integration-tests/all-platforms/blazor_net_8/BlazorTest/wwwroot/bootstrap/bootstrap.min.css.map
go/ql/test/library-tests/semmle/go/frameworks/Twirp/vendor/google.golang.org/protobuf/internal/descfmt/stringer.go
go/ql/test/library-tests/semmle/go/frameworks/Twirp/vendor/google.golang.org/protobuf/internal/errors/errors.go
python/extractor/docs/extractor-python-index.svg
python/extractor/docs/extractor-python-setup.svg
python/extractor/tests/tokenizer/strings.py
python/extractor/tests/tokenizer/strings.token

The ones in minified files, svg files, and the go tests are deliberate, so I didn't change them.

@tausbn Are the non-breaking spaces in python/extractor/tests/tokenizer/strings.py and
python/extractor/tests/tokenizer/strings.token deliberate?

@Copilot Copilot AI review requested due to automatic review settings September 5, 2025 07:42
@aibaars aibaars requested review from a team as code owners September 5, 2025 07:42
Copy link
Contributor

@Copilot 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 non-breaking spaces (Unicode character U+00A0) from code across multiple files and replaces them with regular spaces (U+0020). The changes improve text encoding consistency and prevent potential parsing or formatting issues that can arise from invisible Unicode characters in source code.

Key changes:

  • Fixed whitespace encoding in Ruby, Python, TypeScript, JavaScript, and Go test files
  • Corrected indentation in configuration files and documentation
  • Maintained proper formatting while ensuring consistent space characters

Reviewed Changes

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

Show a summary per file
File Description
ruby/ql/test/query-tests/security/cwe-915/test.rb Fixed line number formatting by replacing non-breaking space
ruby/ql/test/query-tests/experimental/InsecureRandomness/InsecureRandomness.rb Corrected comment indentation with proper spaces
ruby/ql/src/experimental/insecure-randomness/examples/InsecureRandomnessBad.rb Fixed comment line spacing
python/ql/test/library-tests/dataflow/typetracking/attribute_tests.py Replaced non-breaking spaces in multiple comment lines
python/ql/test/library-tests/dataflow/global-flow/known.py Fixed line number formatting
python/extractor/tsg-python/python.tsg Corrected indentation in multiple code blocks
misc/scripts/shared-code-metrics.py Fixed comment formatting
javascript/ql/test/library-tests/TypeScript/RegressionTests/SemicolonInName/test.ts Corrected code indentation
javascript/ql/test/library-tests/TypeScript/RegressionTests/EmptyName/test.ts Fixed code block formatting
javascript/ql/src/experimental/Security/CWE-918/SSRFGood.js Replaced non-breaking space in destructuring assignment
javascript/ql/src/experimental/Security/CWE-918/SSRF.js Fixed variable declaration formatting
javascript/documentation/library-customization.rst Corrected documentation text spacing
go/ql/test/library-tests/semmle/go/frameworks/Revel/EndToEnd.go Fixed comment formatting across multiple lines
go/ql/test/library-tests/semmle/go/frameworks/Beego/test.go Replaced non-breaking space in comment
go/ql/src/Security/CWE-327/InsecureTLS.ql Fixed comment formatting
go/ql/src/InconsistentCode/MissingErrorCheck.ql Corrected comment text spacing
go/ql/src/InconsistentCode/MissingErrorCheck.qhelp Fixed documentation paragraph formatting
go/ql/lib/semmle/go/frameworks/Beego.qll Replaced non-breaking space in comment
go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll Fixed comment formatting
go/ql/lib/semmle/go/Architectures.qll Corrected comment text spacing
go/old-change-notes/2020-10-01-gomod-extraction.md Fixed line formatting
docs/codeql/ql-language-reference/expressions.rst Corrected table cell formatting
actions/ql/src/Security/CWE-829/UntrustedCheckoutMedium.md Fixed comment formatting in code example
.github/workflows/query-list.yml Replaced non-breaking space in comment

@aibaars aibaars added the no-change-note-required This PR does not need a change note label Sep 5, 2025
Copy link
Contributor

github-actions bot commented Sep 5, 2025

QHelp previews:

go/ql/src/InconsistentCode/MissingErrorCheck.qhelp

Missing error check

When a function call returns two values, a pointer and a (subtype of) error, it is conventional to assume that the pointer might be nil until either the pointer or error value has been checked.

If the pointer is dereferenced without a check, an unexpected nil pointer dereference panic may occur.

Recommendation

Ensure that the returned pointer is either directly checked against nil, or the error value is checked before using the returned pointer.

Example

In the example below, user dereferences ptr without checking either ptr or err. This might lead to a panic.

package main

import (
	"fmt"
	"os"
)

func user(input string) {

	ptr, err := os.Open(input)
	// BAD: ptr is dereferenced before either it or `err` has been checked.
	fmt.Printf("Opened %v\n", *ptr)
	if err != nil {
		fmt.Printf("Bad input: %s\n", input)
	}

}

The corrected version of user checks err before using ptr.

package main

import (
	"fmt"
	"os"
)

func user(input string) {

	ptr, err := os.Open(input)
	if err != nil {
		fmt.Printf("Bad input: %s\n", input)
		return
	}
	// GOOD: `err` has been checked before `ptr` is used
	fmt.Printf("Result was %v\n", *ptr)

}

References

@aibaars aibaars merged commit 82476b9 into main Sep 5, 2025
48 of 50 checks passed
@aibaars aibaars deleted the aibaars/drop-nbsp branch September 5, 2025 11:02
@tausbn
Copy link
Contributor

tausbn commented Sep 5, 2025

@tausbn Are the non-breaking spaces in python/extractor/tests/tokenizer/strings.py and
python/extractor/tests/tokenizer/strings.token deliberate?

I had a look through the (ancient) history of these tests, and couldn't find anything that would indicate that this was deliberate. I have no strong opinion about keeping or removing these spaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Actions Analysis of GitHub Actions documentation Go JS no-change-note-required This PR does not need a change note Python Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants