Fix: don't mark JS private class members (e.g. #add) as colors; add tests

[Ayush Kumar (AyushCodes160)]

Issue

Private class members in JavaScript/TypeScript with hex-like names (e.g., #add, #ADD, #abc) are incorrectly displayed with a color picker icon, treating them as CSS color values.

Related: #279225

Root Cause

The hex color detection regex matches valid hex patterns like #add without considering JavaScript's private member syntax. Since #add, #abc, and #dec are all valid 3-character hex values, they get flagged as colors regardless of context.

Solution

Enhanced the hex color detection regex with negative lookahead patterns (?!\s*[\(\{]) to reject matches followed by ( or {, which indicate private class members/methods.

Changes

• src/vs/editor/common/languages/defaultDocumentColorsComputer.ts – Updated regex to skip private member patterns
• src/vs/editor/test/common/languages/defaultDocumentColorsComputer.test.ts – Added 3 new test cases

Testing

All 8 tests passing (5 existing + 3 new)

• Private members #add, #ADD, #abc no longer marked as colors
• Legitimate color detection in strings/objects still works
• No regressions in existing functionality

Fixes #279225

Before

After

[Matt Bierner (mjbvz)]

Aiday Marlen Kyzy (@aiday-mar) Can confirm but I think a better fix would be to only match colors in js files when they appear in strings. That would address a whole class of issues instead just the private members with specific syntax

[Ayush Kumar (AyushCodes160)]

Thanks for the review!
You're right — limiting color detection in JS/TS to strings would solve this more generally. For this PR, I kept the change minimal to fix the reported issue without altering existing behavior.

If the team prefers the broader string-only approach, I’m happy to open a follow-up PR for that.

[Aiday Marlen Kyzy (aiday-mar)]

Hi, thank you for making this PR. I was looking at it and noticed you changed the regex pattern we use to detect the default color documents and you added a method which checks that the color is not followed by a parenthesis or bracket. I thought the regex change is enough. Why was the method added? Is this necessary, and if yes could you walk me through what it does that the regex does not?

[Ayush Kumar (AyushCodes160)]

Aiday Marlen Kyzy (@aiday-mar)
Thanks for the review. You’re correct that the updated regex addresses the primary issue by preventing matches where a hex literal continues into an identifier or is followed by ( or {, which covers the majority of private class member cases.

The helper was added intentionally as a second-stage validation because the regex operates purely on local pattern constraints, while the ambiguity here is contextual. Certain sequences like #abc( or #ADD { are syntactically valid hex colors and valid JavaScript private member declarations. While the regex filters these based on lookaheads, the helper explicitly inspects the character immediately following the match in the full document text to confirm it matches member-definition patterns (method/property), rather than a color literal usage.

In other words:

Regex: prevents most false positives by enforcing boundary and lookahead constraints.

Helper: disambiguates remaining edge cases where valid hex values overlap with valid JS private identifiers and are immediately followed by member syntax.

That said, with the current regex lookaheads in place, the helper likely does not add additional coverage and could be considered redundant. I’m happy to remove it and keep the logic regex-only if that’s the preferred approach.

[Aiday Marlen Kyzy (aiday-mar)]

Thanks Ayush Kumar (@AyushCodes160) could you remove the method and keep the fix regex based? We can then merge the PR.

[vs-code-engineering]

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

Raymond Zhao (@rzhao271)

Matched files:

• src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts

[Ayush Kumar (AyushCodes160)]

Aiday Marlen Kyzy (@aiday-mar) ma'am, I've removed the _isPrivateJavaScriptMember function and now rely solely on the improved regex for color detection, as requested. All tests pass and private JS class members (e.g. #add) are no longer detected as colors. Please let me know if any further changes are needed!

[Copilot]

Pull request overview

This PR fixes an issue where JavaScript/TypeScript private class members with hex-like names (e.g., #add, #ABC, #abc) were incorrectly being flagged as CSS color values and displayed with color picker decorations. The fix enhances the hex color detection regex with negative lookahead patterns to exclude matches followed by ( or {, which indicate private member syntax.

Key Changes

• Enhanced regex pattern in color detection to avoid matching JavaScript private members
• Added comprehensive test coverage for private member edge cases
• Included unrelated defensive fix in settings editor (should be in separate PR)

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/vs/editor/common/languages/defaultDocumentColorsComputer.ts	Updated hex color regex with negative lookahead to exclude private member patterns
src/vs/editor/test/common/languages/defaultDocumentColorsComputer.test.ts	Added 3 test cases covering private members, hex-like names, and color detection after operators
src/vs/workbench/contrib/preferences/browser/settingsEditor2.ts	Added defensive check before revealing/focusing tree elements (unrelated to color fix)
package.json	Added tsx development dependency (appears to be for temporary testing)
package-lock.json	Lock file updates for tsx and its dependencies
tmp/checkColors.mjs	Temporary test script (should not be committed)

[Copilot]

The regex pattern has inconsistent negative lookahead patterns across different hex color lengths. Some patterns use (?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{]) while others use (?!\s*[\(\{]). This inconsistency means that for some hex patterns (3-char and 4-char with lookbehind), the pattern prevents matching when followed by identifier characters OR parentheses/braces, but for others (4-char, 6-char, 8-char starting with ^), it only prevents matching when followed by parentheses/braces. This could lead to false positives where valid hex colors at the start of a line followed by identifier characters are still matched. Consider applying the more comprehensive negative lookahead (?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{]) consistently to all hex color patterns to ensure uniform behavior.

Suggested change

const initialValidationRegex = /\b(rgb|rgba|hsl|hsla)(\([0-9\s,\.\%]*\))|^(#)([A-Fa-f0-9]{3})(?!\s*[\(\{])\b|^(#)([A-Fa-f0-9]{4})(?!\s*[\(\{])\b|^(#)([A-Fa-f0-9]{6})(?!\s*[\(\{])\b|^(#)([A-Fa-f0-9]{8})(?!\s*[\(\{])\b|(?<=['"\s])(#)([A-Fa-f0-9]{3})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])|(?<=['"\s])(#)([A-Fa-f0-9]{4})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])|(?<=['"\s])(#)([A-Fa-f0-9]{6})(?!\s*[\(\{])\b|(?<=['"\s])(#)([A-Fa-f0-9]{8})(?!\s*[\(\{])\b/gm;

const initialValidationRegex = /\b(rgb|rgba|hsl|hsla)(\([0-9\s,\.\%]*\))|^(#)([A-Fa-f0-9]{3})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])\b|^(#)([A-Fa-f0-9]{4})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])\b|^(#)([A-Fa-f0-9]{6})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])\b|^(#)([A-Fa-f0-9]{8})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])\b|(?<=['"\s])(#)([A-Fa-f0-9]{3})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])|(?<=['"\s])(#)([A-Fa-f0-9]{4})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])|(?<=['"\s])(#)([A-Fa-f0-9]{6})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])\b|(?<=['"\s])(#)([A-Fa-f0-9]{8})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])\b/gm;

[Copilot]

The comment references issue #231852, but the PR description mentions issue #279225. Please ensure the issue number in the comment is correct, or update it to match the issue being fixed.

Suggested change

	// Test case from issue #231852: Private members like #add should not be marked as colors
	// Test case from issue #279225: Private members like #add should not be marked as colors

[Copilot]

The tsx package is being added as a devDependency, but it's not clear from the PR description why this package is needed. If tsx was only used for temporary testing (e.g., to run tmp/checkColors.mjs), it should be removed along with the temporary test file. If it's intended for permanent use in the project, please add a note to the PR description explaining its purpose.

Suggested change

"tsx": "^4.20.6",

[Copilot]

This change to settingsEditor2.ts appears unrelated to the PR's stated purpose of fixing color detection for JavaScript private class members. While this defensive check (ensuring the element exists before reveal/setFocus) may be a valid bug fix, it should either be explained in the PR description or submitted as a separate PR for better traceability and review focus.

[Copilot]

The tmp/checkColors.mjs file appears to be a temporary test/debug script that should not be committed to the repository. Temporary development files should be cleaned up before merging the PR to keep the repository clean.

Suggested change

	import { computeDefaultDocumentColors } from 'file:///Users/ayushkumar/Desktop/vscode/out/vs/editor/common/languages/defaultDocumentColorsComputer.js';
	const content = `class Calc {
	#add(a, b) { return a + b; }
	#sub(a, b) { return a - b; }
	}`;
	const model = {
	getValue: () => content,
	positionAt: (offset) => {
	const lines = content.substring(0, offset).split('\n');
	return { lineNumber: lines.length, column: lines[lines.length - 1].length + 1 };
	},
	findMatches: (regex) => [...content.matchAll(regex)]
	};
	const colors = computeDefaultDocumentColors(model);
	console.log('matches:', JSON.stringify(colors, null, 2));
	console.log('matches length:', colors.length);
	// Also test a string that contains #ADD etc
	const content2 = `class Calc {\n #ADD(a,b) { return a+b; }\n #abc(a,b) { return a-b; }\n}`;
	const model2 = { getValue: () => content2, positionAt: (offset) => { const lines = content2.substring(0, offset).split('\n'); return { lineNumber: lines.length, column: lines[lines.length - 1].length + 1 }; }, findMatches: (regex) => [...content2.matchAll(regex)] };
	console.log('matches2:', JSON.stringify(computeDefaultDocumentColors(model2), null, 2));
	console.log('matches2 length:', computeDefaultDocumentColors(model2).length);

[Copilot]

The multi-line comment explaining the regex uses redundant phrases. Lines 105-108 repeat similar explanations about preventing matches with private JavaScript identifiers. This could be condensed into a single, clearer statement like: "Use negative lookahead to prevent matching JavaScript private member syntax (#identifier) by ensuring hex colors don't continue into identifier characters or are followed by ( or {."

Suggested change

	// Note: Use negative lookahead (?![A-Fa-f0-9_\w]) to prevent matching private JavaScript identifiers like #add or #ADD
	// Private member names in JS are written as #identifier, so we ensure hex colors don't continue into valid identifier characters
	// For hex colors to be valid, they must end at a word/identifier boundary to avoid matching private member names like #add.
	// Use negative lookahead to reject colors that have identifier characters after them.
	// Use negative lookahead to prevent matching JavaScript private member syntax (#identifier) by ensuring hex colors don't continue into identifier characters or are followed by ( or {.

[Copilot]

The test coverage is missing edge cases for private field declarations without parentheses. For example, #color = '#fff'; contains both a private field name and a legitimate hex color value. The current tests only cover private methods (with parentheses) and should also verify that private field assignments correctly distinguish between the field name and any color values.

[Copilot]

Consider adding test coverage for TypeScript-specific syntax like static private members (static #abc() {}) and private accessor patterns (get #abc() {}, set #abc(val) {}). While the current regex should handle these since it checks for ( and {, explicit tests would ensure these edge cases remain covered as the code evolves.

[Copilot]

The complex regex with multiple alternations and lookahead/lookbehind assertions could have performance implications on large files. Consider breaking this into separate, simpler regex patterns or adding comments about the expected performance characteristics. The regex engine needs to backtrack through multiple alternatives, which could be slow on files with many potential matches.

Suggested change

	// Note: Use negative lookahead (?![A-Fa-f0-9_\w]) to prevent matching private JavaScript identifiers like #add or #ADD
	// Private member names in JS are written as #identifier, so we ensure hex colors don't continue into valid identifier characters
	// For hex colors to be valid, they must end at a word/identifier boundary to avoid matching private member names like #add.
	// Use negative lookahead to reject colors that have identifier characters after them.
	const initialValidationRegex = /\b(rgb|rgba|hsl|hsla)(\([0-9\s,\.\%]*\))|^(#)([A-Fa-f0-9]{3})(?!\s*[\(\{])\b|^(#)([A-Fa-f0-9]{4})(?!\s*[\(\{])\b|^(#)([A-Fa-f0-9]{6})(?!\s*[\(\{])\b|^(#)([A-Fa-f0-9]{8})(?!\s*[\(\{])\b|(?<=['"\s])(#)([A-Fa-f0-9]{3})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])|(?<=['"\s])(#)([A-Fa-f0-9]{4})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])|(?<=['"\s])(#)([A-Fa-f0-9]{6})(?!\s*[\(\{])\b|(?<=['"\s])(#)([A-Fa-f0-9]{8})(?!\s*[\(\{])\b/gm;
	const initialValidationMatches = _findMatches(model, initialValidationRegex);
	// The original regex was very complex and could cause performance issues due to excessive backtracking.
	// To improve performance and maintainability, we split the regex into several simpler patterns, each targeting a specific color format.
	// This reduces the risk of catastrophic backtracking and makes the code easier to maintain.
	// Regex for rgb(), rgba(), hsl(), hsla()
	const rgbHslRegex = /\b(rgb|rgba|hsl|hsla)(\([0-9\s,\.\%]*\))/gm;
	// Regexes for hex colors (3, 4, 6, 8 digits), with and without lookbehind
	// Note: We use two sets: one for start-of-line, one for after quote/whitespace (using lookbehind)
	const hex3Regex = /^(#)([A-Fa-f0-9]{3})(?!\s*[\(\{])\b/gm;
	const hex4Regex = /^(#)([A-Fa-f0-9]{4})(?!\s*[\(\{])\b/gm;
	const hex6Regex = /^(#)([A-Fa-f0-9]{6})(?!\s*[\(\{])\b/gm;
	const hex8Regex = /^(#)([A-Fa-f0-9]{8})(?!\s*[\(\{])\b/gm;
	// Lookbehind for quote or whitespace (ES2018+)
	const hex3LookbehindRegex = /(?<=['"\s])(#)([A-Fa-f0-9]{3})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])/gm;
	const hex4LookbehindRegex = /(?<=['"\s])(#)([A-Fa-f0-9]{4})(?![A-Fa-f0-9a-zA-Z_]|\s*[\(\{])/gm;
	const hex6LookbehindRegex = /(?<=['"\s])(#)([A-Fa-f0-9]{6})(?!\s*[\(\{])\b/gm;
	const hex8LookbehindRegex = /(?<=['"\s])(#)([A-Fa-f0-9]{8})(?!\s*[\(\{])\b/gm;
	// Collect all matches from all regexes
	const initialValidationMatches: RegExpMatchArray[] = [
	..._findMatches(model, rgbHslRegex),
	..._findMatches(model, hex3Regex),
	..._findMatches(model, hex4Regex),
	..._findMatches(model, hex6Regex),
	..._findMatches(model, hex8Regex),
	..._findMatches(model, hex3LookbehindRegex),
	..._findMatches(model, hex4LookbehindRegex),
	..._findMatches(model, hex6LookbehindRegex),
	..._findMatches(model, hex8LookbehindRegex)
	];

[Aiday Marlen Kyzy (aiday-mar)]

I left comments on this PR.

[Aiday Marlen Kyzy (aiday-mar)]

The package.json should not change

[Aiday Marlen Kyzy (aiday-mar)]

The package.json should not change

[Aiday Marlen Kyzy (aiday-mar)]

You can make the comment shorter, it is quite long.

[Aiday Marlen Kyzy (aiday-mar)]

Why has this been changed?

[Aiday Marlen Kyzy (aiday-mar)]

What is this file? It should not be added.

[microsoft-github-policy-service]

Ayush Kumar (@AyushCodes160) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

• (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

@microsoft-github-policy-service agree

• (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

Contribution License Agreement

This Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
and conveys certain license rights to Microsoft Corporation and its affiliates (“Microsoft”) for Your
contributions to Microsoft open source projects. This Agreement is effective as of the latest signature
date below.

1. Definitions.
   “Code” means the computer software code, whether in human-readable or machine-executable form,
   that is delivered by You to Microsoft under this Agreement.
   “Project” means any of the projects owned or managed by Microsoft and offered under a license
   approved by the Open Source Initiative (www.opensource.org).
   “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any
   Project, including but not limited to communication on electronic mailing lists, source code control
   systems, and issue tracking systems that are managed by, or on behalf of, the Project for the purpose of
   discussing and improving that Project, but excluding communication that is conspicuously marked or
   otherwise designated in writing by You as “Not a Submission.”
   “Submission” means the Code and any other copyrightable material Submitted by You, including any
   associated comments and documentation.
2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any
   Project. This Agreement covers any and all Submissions that You, now or in the future (except as
   described in Section 4 below), Submit to any Project.
3. Originality of Work. You represent that each of Your Submissions is entirely Your original work.
   Should You wish to Submit materials that are not Your original work, You may Submit them separately
   to the Project if You (a) retain all copyright and license information that was in the materials as You
   received them, (b) in the description accompanying Your Submission, include the phrase “Submission
   containing materials of a third party:” followed by the names of the third party and any licenses or other
   restrictions of which You are aware, and (c) follow any other instructions in the Project’s written
   guidelines concerning Submissions.
4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else
   for whom You are acting in making Your Submission, e.g. as a contractor, vendor, or agent. If Your
   Submission is made in the course of Your work for an employer or Your employer has intellectual
   property rights in Your Submission by contract or applicable law, You must secure permission from Your
   employer to make the Submission before signing this Agreement. In that case, the term “You” in this
   Agreement will refer to You and the employer collectively. If You change employers in the future and
   desire to Submit additional Submissions for the new employer, then You agree to sign a new Agreement
   and secure permission from the new employer before Submitting those Submissions.
5. Licenses.

• Copyright License. You grant Microsoft, and those who receive the Submission directly or
  indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license in the
  Submission to reproduce, prepare derivative works of, publicly display, publicly perform, and distribute
  the Submission and such derivative works, and to sublicense any or all of the foregoing rights to third
  parties.
• Patent License. You grant Microsoft, and those who receive the Submission directly or
  indirectly from Microsoft, a perpetual, worldwide, non-exclusive, royalty-free, irrevocable license under
  Your patent claims that are necessarily infringed by the Submission or the combination of the
  Submission with the Project to which it was Submitted to make, have made, use, offer to sell, sell and
  import or otherwise dispose of the Submission alone or with the Project.
• Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement.
  No additional licenses or rights whatsoever (including, without limitation, any implied licenses) are
  granted by implication, exhaustion, estoppel or otherwise.

6. Representations and Warranties. You represent that You are legally entitled to grant the above
   licenses. You represent that each of Your Submissions is entirely Your original work (except as You may
   have disclosed under Section 3). You represent that You have secured permission from Your employer to
   make the Submission in cases where Your Submission is made in the course of Your work for Your
   employer or Your employer has intellectual property rights in Your Submission by contract or applicable
   law. If You are signing this Agreement on behalf of Your employer, You represent and warrant that You
   have the necessary authority to bind the listed employer to the obligations contained in this Agreement.
   You are not expected to provide support for Your Submission, unless You choose to do so. UNLESS
   REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING, AND EXCEPT FOR THE WARRANTIES
   EXPRESSLY STATED IN SECTIONS 3, 4, AND 6, THE SUBMISSION PROVIDED UNDER THIS AGREEMENT IS
   PROVIDED WITHOUT WARRANTY OF ANY KIND, INCLUDING, BUT NOT LIMITED TO, ANY WARRANTY OF
   NONINFRINGEMENT, MERCHANTABILITY, OR FITNESS FOR A PARTICULAR PURPOSE.
7. Notice to Microsoft. You agree to notify Microsoft in writing of any facts or circumstances of which
   You later become aware that would make Your representations in this Agreement inaccurate in any
   respect.
8. Information about Submissions. You agree that contributions to Projects and information about
   contributions may be maintained indefinitely and disclosed publicly, including Your name and other
   information that You submit with Your Submission.
9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and
   the parties consent to exclusive jurisdiction and venue in the federal courts sitting in King County,
   Washington, unless no federal subject matter jurisdiction exists, in which case the parties consent to
   exclusive jurisdiction and venue in the Superior Court of King County, Washington. The parties waive all
   defenses of lack of personal jurisdiction and forum non-conveniens.
10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and
    supersedes any and all prior agreements, understandings or communications, written or oral, between
    the parties relating to the subject matter hereof. This Agreement may be assigned by Microsoft.

[Ayush Kumar (AyushCodes160)]

Apologies for the confusion ma'am ,while working on this branch, I accidentally added PR_SUMMARY.md, which was intended for a college assignment and not related to this project. I have now removed the unwanted file and restored the relevant code as discussed. If you notice any other issues or have further feedback, please let me know. Thank you for your review and guidance!

[Aiday Marlen Kyzy (aiday-mar)]

Hi Ayush Kumar (@AyushCodes160) thanks for the comment. Could you please resolve all of the comments I left under #280085 (review)? I see there are still comments to be resolved. Additionally there is a merge conflict you should fix.