From 45c249760e2592d373bf3b8fbc9b46ed94c613a1 Mon Sep 17 00:00:00 2001 From: abose Date: Tue, 30 Jun 2026 20:42:24 +0530 Subject: [PATCH 1/5] feat(codehints): language-agnostic "doc comment" generator Adds a DocCommentHints default extension: when the caret is on a doc-comment opener above a declaration, it offers a single hint - "Add JSDoc comment" / "Add docstring" - that expands a Tab-navigable skeleton (@param per parameter + @returns, or Python Args:/Returns:) with the parameters parsed from the signature. - Works across JS/TS/JSX/TSX/PHP/Java/C/C++/Rust (JSDoc) and Python (docstring); names come from parsing the declaration, so it needs no language server, with a clean seam to enrich types from LSP/Tern later. - Registers at high priority but claims hints ONLY in the opener context, so it never shadows the normal LSP/Tern completion providers. - Triggers on a half-typed opener (/, /*) too, gated on an adjacent declaration so a stray / never spams the list; re-validates each keystroke so it dismisses cleanly. - Generated lines carry no trailing whitespace (tabstops sit on the {type} only), so linters stay quiet. - TabstopManager is preloaded in brackets.js / SpecRunner.js so the extension can resolve it synchronously. Unit tests (unit:DocCommentHints) cover the param parser, the per-language name conventions, and trailing-whitespace-free snippet output. --- src/brackets.js | 2 + src/extensions/default/DefaultExtensions.json | 1 + .../default/DocCommentHints/main.js | 355 ++++++++++++++++++ .../default/DocCommentHints/package.json | 6 + .../DocCommentHints/requirejs-config.json | 1 + .../default/DocCommentHints/unittests.js | 162 ++++++++ src/nls/root/strings.js | 4 + src/styles/brackets.less | 11 + test/SpecRunner.js | 2 + 9 files changed, 544 insertions(+) create mode 100644 src/extensions/default/DocCommentHints/main.js create mode 100644 src/extensions/default/DocCommentHints/package.json create mode 100644 src/extensions/default/DocCommentHints/requirejs-config.json create mode 100644 src/extensions/default/DocCommentHints/unittests.js diff --git a/src/brackets.js b/src/brackets.js index e6adcb0756..bdb2d060b9 100644 --- a/src/brackets.js +++ b/src/brackets.js @@ -197,6 +197,8 @@ define(function (require, exports, module) { require("editor/EditorOptionHandlers"); require("editor/EditorStatusBar"); require("editor/ImageViewer"); + // Preload so extensions can synchronously brackets.getModule it (e.g. DocCommentHints snippets). + require("editor/TabstopManager"); require("extensibility/ExtensionDownloader"); require("extensibility/InstallExtensionDialog"); require("extensibility/ExtensionManagerDialog"); diff --git a/src/extensions/default/DefaultExtensions.json b/src/extensions/default/DefaultExtensions.json index b1d9deb531..7076ad7306 100644 --- a/src/extensions/default/DefaultExtensions.json +++ b/src/extensions/default/DefaultExtensions.json @@ -8,6 +8,7 @@ "CSSCodeHints", "CSSPseudoSelectorHints", "DebugCommands", + "DocCommentHints", "HandlebarsSupport", "HTMLCodeHints", "HtmlEntityCodeHints", diff --git a/src/extensions/default/DocCommentHints/main.js b/src/extensions/default/DocCommentHints/main.js new file mode 100644 index 0000000000..b9731f08fe --- /dev/null +++ b/src/extensions/default/DocCommentHints/main.js @@ -0,0 +1,355 @@ +/* + * GNU AGPL-3.0 License + * + * Copyright (c) 2021 - present core.ai . All rights reserved. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License + * for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://opensource.org/licenses/AGPL-3.0. + * + */ + +/** + * DocCommentHints - a language-agnostic "generate documentation comment" code hint. + * + * When the caret sits on a freshly typed doc-comment opener (`/**` for the JSDoc/Javadoc family, + * `"""` for Python) the provider offers a single hint. Accepting it reads the declaration on the + * next line, extracts its parameters, and expands a doc-comment skeleton (with Tab-navigable fields) + * in that language's style. + * + * It is NOT tied to any one language server: the parameter names come from parsing the declaration, + * so it works wherever the editor knows the language - and degrades to a bare skeleton when the + * declaration can't be parsed. The signature source is deliberately a single seam (_parseSignature), + * so a future enhancement can enrich types from an LSP/Tern when one is available for the language. + * + * The provider registers at a high priority but claims hints ONLY in the opener context, so it never + * shadows the normal completion providers (LSP/Tern) anywhere else. + */ +define(function (require, exports, module) { + + + const AppInit = brackets.getModule("utils/AppInit"), + CodeHintManager = brackets.getModule("editor/CodeHintManager"), + EditorManager = brackets.getModule("editor/EditorManager"), + TabstopManager = brackets.getModule("editor/TabstopManager"), + Strings = brackets.getModule("strings"); + + // Above the LSP (1) and Tern (0) hint providers - safe because hasHints only claims the opener + // context, where those providers have nothing useful to offer anyway. + const PROVIDER_PRIORITY = 100; + + const STYLE_JSDOC = "jsdoc", + STYLE_PYDOC = "pydoc"; + + // Per-language config: the doc-comment style, where a parameter's NAME sits in a declaration + // ("first" -> `name: Type` / `name` / `$name`; "last" -> `Type name`, the C/Java family), and + // which opener triggers the hint. + const LANGUAGES = { + javascript: { style: STYLE_JSDOC, params: "first", opener: "block" }, + typescript: { style: STYLE_JSDOC, params: "first", opener: "block" }, + jsx: { style: STYLE_JSDOC, params: "first", opener: "block" }, + tsx: { style: STYLE_JSDOC, params: "first", opener: "block" }, + php: { style: STYLE_JSDOC, params: "first", opener: "block" }, + rust: { style: STYLE_JSDOC, params: "first", opener: "block" }, + java: { style: STYLE_JSDOC, params: "last", opener: "block" }, + c: { style: STYLE_JSDOC, params: "last", opener: "block" }, + cpp: { style: STYLE_JSDOC, params: "last", opener: "block" }, + python: { style: STYLE_PYDOC, params: "first", opener: "pydoc" } + }; + + // Opener: `full` is the canonical opener (offered unconditionally); `partial` also matches the + // half-typed forms (`/`, `/*`) so the hint can appear as soon as the user starts the comment - + // but a partial match is only offered when a documentable declaration sits next to it (see + // _openerContext), so a stray `/` on a line never spams the list. `chars` are the keystrokes that + // may trigger an implicit request; `dir` is where the documented declaration lives relative to the + // opener (below it for block comments, above it for a Python docstring). + const OPENERS = { + block: { full: /^\s*\/\*\*$/, partial: /^\s*\/\*{0,2}$/, chars: "/*", dir: "below" }, + pydoc: { full: /^\s*"""$/, partial: /^\s*"""$/, chars: "\"", dir: "above" } + }; + + function _configFor(editor) { + return LANGUAGES[editor.getLanguageForSelection().getId()] || null; + } + + // ----- signature parsing ----------------------------------------------------------------- + + function _delta(ch) { + if (ch === "(" || ch === "[" || ch === "{" || ch === "<") { return 1; } + if (ch === ")" || ch === "]" || ch === "}" || ch === ">") { return -1; } + return 0; + } + + // The declaration the doc comment documents, joining wrapped lines until its parameter parens + // balance. `dir` is "below" (block comments sit above the declaration) or "above" (a Python + // docstring sits inside, just under the `def`). Returns the declaration text, or null if none. + function _declarationFor(editor, openerLine, dir) { + const lastLine = editor.lineCount() - 1; + const step = dir === "above" ? -1 : 1; + let l = openerLine + step; + while (l >= 0 && l <= lastLine && editor.document.getLine(l).trim() === "") { + l += step; + } + if (l < 0 || l > lastLine) { + return null; + } + let text = editor.document.getLine(l); + let guard = 0; + // A wrapped signature leaves the parens unbalanced; pull in adjacent lines until they close. + // (Below: extra "(" to the right; above: extra ")" to the left.) + while (_parenDepth(text) !== 0 && l > 0 && l < lastLine && guard < 30) { + l += step; + text = step > 0 ? text + " " + editor.document.getLine(l) : editor.document.getLine(l) + " " + text; + guard++; + } + return text; + } + + function _parenDepth(text) { + let depth = 0; + for (let i = 0; i < text.length; i++) { + if (text[i] === "(") { depth++; } else if (text[i] === ")") { depth--; } + } + return depth; + } + + // Split a parameter list on top-level commas, ignoring commas nested in (), [], {}, <>. + function _splitParams(s) { + const out = []; + let depth = 0, start = 0; + for (let i = 0; i < s.length; i++) { + depth += _delta(s[i]); + if (s[i] === "," && depth === 0) { + out.push(s.slice(start, i)); + start = i + 1; + } + } + if (s.slice(start).trim() !== "") { + out.push(s.slice(start)); + } + return out; + } + + const IDENT = /[A-Za-z_][\w]*/g; + const PARAMS_TO_SKIP = { self: true, cls: true, this: true, void: true }; + + // Extract a clean parameter name from one parameter token per the language's name convention. + function _paramName(token, convention) { + let t = token.trim(); + if (!t) { + return null; + } + // Drop a default value (top-level '='). + let depth = 0; + for (let i = 0; i < t.length; i++) { + depth += _delta(t[i]); + if (t[i] === "=" && depth === 0) { + t = t.slice(0, i); + break; + } + } + t = t.replace(/\.\.\./g, " ").trim(); // rest/spread + if (convention === "first") { + // `name`, `name: Type`, `$name` (PHP). Keep a leading $ if present. + const m = t.match(/^[*&\s]*(\$?[A-Za-z_][\w]*)/); + return m ? m[1] : null; + } + // "last": C/Java `Type name`, `const char *name` -> the trailing identifier is the name. + const ids = t.match(IDENT); + return ids && ids.length ? ids[ids.length - 1] : null; + } + + /** + * Parse a declaration line into a documentable signature. `isDeclaration` is true only when the + * text actually looks like something to document (a function/method - has a parameter list - or a + * class), which is what gates the half-typed `/` `/*` triggers. + * @return {?{params: string[], isClass: boolean, hasReturn: boolean, isDeclaration: boolean}} + */ + function _parseSignature(declText, convention) { + if (!declText) { + return null; + } + const open = declText.indexOf("("); + const classMatch = /\b(class|interface|struct|enum|trait)\b/.exec(declText); + if (classMatch && (open === -1 || classMatch.index < open)) { + return { params: [], isClass: true, hasReturn: false, isDeclaration: true }; + } + if (open === -1) { + return { params: [], isClass: false, hasReturn: false, isDeclaration: false }; + } + let depth = 0, close = -1; + for (let i = open; i < declText.length; i++) { + if (declText[i] === "(") { + depth++; + } else if (declText[i] === ")") { + depth--; + if (depth === 0) { close = i; break; } + } + } + const inner = close === -1 ? declText.slice(open + 1) : declText.slice(open + 1, close); + const params = []; + _splitParams(inner).forEach(function (tok) { + const name = _paramName(tok, convention); + if (name && !PARAMS_TO_SKIP[name]) { + params.push(name); + } + }); + // Return: a constructor returns nothing; an explicit `void`/`None`/`-> ()` return type is void. + const after = close === -1 ? "" : declText.slice(close + 1); + const isCtor = /\b(constructor|__init__)\b/.test(declText) || /\bvoid\s+\w+\s*\(/.test(declText); + const isVoid = /:\s*void\b/.test(after) || /->\s*(None|\(\s*\))/.test(after) || isCtor; + return { params: params, isClass: false, hasReturn: !isVoid, isDeclaration: true }; + } + + // ----- snippet building ------------------------------------------------------------------ + + // Escape text inserted literally into an LSP-style snippet ($ } \ are special). + function _esc(text) { + return String(text).replace(/[\\$}]/g, "\\$&"); + } + + function _buildJsDoc(sig, indent) { + const star = indent + " * "; + const out = ["/**", star + "${1:" + _escDesc(Strings.DOC_COMMENT_SUMMARY) + "}"]; + let stop = 2; + if (sig && !sig.isClass) { + // Tabstop on the {type} only; no trailing description stub (an empty tabstop would leave a + // trailing space that linters flag). Descriptions are added inline by the user. + sig.params.forEach(function (p) { + out.push(star + "@param {${" + (stop++) + ":*}} " + _esc(p)); + }); + if (sig.hasReturn) { + out.push(star + "@returns {${" + (stop++) + ":*}}"); + } + } + out.push(indent + " */"); + return out.join("\n"); + } + + function _buildPyDoc(sig, indent) { + const out = ['"""${1:' + _escDesc(Strings.DOC_COMMENT_SUMMARY) + "}"]; + let stop = 2; + const desc = _escDesc(Strings.DOC_COMMENT_DESC); + if (sig && sig.params.length) { + out.push(""); + out.push(indent + "Args:"); + // Non-empty placeholder (selected on tab) so the line carries no trailing whitespace. + sig.params.forEach(function (p) { + out.push(indent + " " + _esc(p) + ": ${" + (stop++) + ":" + desc + "}"); + }); + } + if (sig && sig.hasReturn) { + out.push(""); + out.push(indent + "Returns:"); + out.push(indent + " ${" + (stop++) + ":" + desc + "}"); + } + out.push(indent + '"""'); + return out.join("\n"); + } + + // The summary placeholder is literal default text inside ${1:...} - only } and \ need escaping. + function _escDesc(text) { + return String(text).replace(/[\\}]/g, "\\$&"); + } + + function _buildSnippet(style, sig, indent) { + return style === STYLE_PYDOC ? _buildPyDoc(sig, indent) : _buildJsDoc(sig, indent); + } + + // ----- the hint provider ----------------------------------------------------------------- + + // Decide whether the caret is in a doc-comment-opener context worth offering the hint for, and + // gather the declaration it would document. The full opener (`/**`, `"""`) is always offered; a + // half-typed opener (`/`, `/*`) is offered only when an actual declaration sits next to it. + // `implicitChar` is the just-typed char (null for an explicit/refresh request). + function _openerContext(editor, implicitChar) { + const cfg = _configFor(editor); + if (!cfg) { + return null; + } + const opener = OPENERS[cfg.opener]; + if (implicitChar && opener.chars.indexOf(implicitChar) === -1) { + return null; + } + const cursor = editor.getCursorPos(); + const before = editor.document.getLine(cursor.line).slice(0, cursor.ch); + if (opener.full.test(before)) { + return { cfg: cfg }; + } + if (opener.partial.test(before)) { + const sig = _parseSignature(_declarationFor(editor, cursor.line, opener.dir), cfg.params); + if (sig && sig.isDeclaration) { + return { cfg: cfg }; + } + } + return null; + } + + function DocCommentHintProvider() {} + + DocCommentHintProvider.prototype.hasHints = function (editor, implicitChar) { + this.editor = editor; + const ctx = _openerContext(editor, implicitChar); + this._style = ctx && ctx.cfg.style; + return !!ctx; + }; + + DocCommentHintProvider.prototype.getHints = function () { + // Re-validate: end the session if the caret moved out of the opener context (e.g. the user + // kept typing past `/`), so the hint doesn't linger inappropriately. + if (!this.editor || !_openerContext(this.editor, null)) { + return null; + } + // A clear, language-aware action label - not a cryptic "/**/". The leading marker echoes the + // doc-comment syntax so it reads as "insert a doc comment here". + const isPy = this._style === STYLE_PYDOC; + const label = isPy ? Strings.DOC_COMMENT_ADD_DOCSTRING : Strings.DOC_COMMENT_ADD_JSDOC; + const $hint = $("") + .addClass("doc-comment-hint") + .data("docComment", true) + .append($("").addClass("doc-comment-hint-marker").text(isPy ? '"""' : "/**")) + .append($("").addClass("doc-comment-hint-label").text(" " + label)); + return { hints: [$hint], match: null, selectInitial: true, handleWideResults: true }; + }; + + DocCommentHintProvider.prototype.insertHint = function () { + const editor = this.editor || EditorManager.getActiveEditor(); + const cfg = editor && _configFor(editor); + if (!editor || !cfg) { + return false; + } + const cursor = editor.getCursorPos(); + const line = editor.document.getLine(cursor.line); + const indent = (line.match(/^\s*/) || [""])[0]; + + const decl = _declarationFor(editor, cursor.line, OPENERS[cfg.opener].dir); + const sig = _parseSignature(decl, cfg.params); + const snippet = _buildSnippet(cfg.style, sig, indent); + + // Replace the opener the user typed (from the start of `/**` / `"""` up to the caret). + const startPos = { line: cursor.line, ch: indent.length }; + const endPos = { line: cursor.line, ch: cursor.ch }; + TabstopManager.insertSnippet(editor, snippet, startPos, endPos); + return false; + }; + + AppInit.appReady(function () { + const provider = new DocCommentHintProvider(); + CodeHintManager.registerHintProvider(provider, Object.keys(LANGUAGES), PROVIDER_PRIORITY); + }); + + // Exposed for unit tests. + exports._parseSignature = _parseSignature; + exports._buildSnippet = _buildSnippet; + exports._splitParams = _splitParams; + exports._paramName = _paramName; +}); diff --git a/src/extensions/default/DocCommentHints/package.json b/src/extensions/default/DocCommentHints/package.json new file mode 100644 index 0000000000..8ab3def9c6 --- /dev/null +++ b/src/extensions/default/DocCommentHints/package.json @@ -0,0 +1,6 @@ +{ + "name": "DocCommentHints", + "version": "1.0.0", + "dependencies": { + } +} diff --git a/src/extensions/default/DocCommentHints/requirejs-config.json b/src/extensions/default/DocCommentHints/requirejs-config.json new file mode 100644 index 0000000000..9e26dfeeb6 --- /dev/null +++ b/src/extensions/default/DocCommentHints/requirejs-config.json @@ -0,0 +1 @@ +{} \ No newline at end of file diff --git a/src/extensions/default/DocCommentHints/unittests.js b/src/extensions/default/DocCommentHints/unittests.js new file mode 100644 index 0000000000..dabd824566 --- /dev/null +++ b/src/extensions/default/DocCommentHints/unittests.js @@ -0,0 +1,162 @@ +/* + * GNU AGPL-3.0 License + * + * Copyright (c) 2021 - present core.ai . All rights reserved. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License + * for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://opensource.org/licenses/AGPL-3.0. + * + */ + +/*global describe, it, expect*/ + +define(function (require, exports, module) { + const DocCommentHints = require("./main"); + + describe("unit:DocCommentHints", function () { + + describe("_splitParams - top-level comma split", function () { + const split = DocCommentHints._splitParams; + it("splits simple params", function () { + expect(split("a, b, c").map(s => s.trim())).toEqual(["a", "b", "c"]); + }); + it("ignores commas inside braces/brackets/parens/angles", function () { + expect(split("a, {x, y}, b").map(s => s.trim())).toEqual(["a", "{x, y}", "b"]); + expect(split("a, f(b, c), d").map(s => s.trim())).toEqual(["a", "f(b, c)", "d"]); + expect(split("a: Map, b").map(s => s.trim())) + .toEqual(["a: Map", "b"]); + }); + it("returns nothing for an empty list", function () { + expect(split("")).toEqual([]); + expect(split(" ")).toEqual([]); + }); + }); + + describe("_paramName - per-convention name extraction", function () { + const name = DocCommentHints._paramName; + it("name-first: plain, typed, default, PHP, rest", function () { + expect(name("name", "first")).toBe("name"); + expect(name("name: string", "first")).toBe("name"); + expect(name("count = 5", "first")).toBe("count"); + expect(name("count: number = 1", "first")).toBe("count"); + expect(name("$user", "first")).toBe("$user"); + expect(name("...rest", "first")).toBe("rest"); + }); + it("type-first (C/Java): trailing identifier is the name", function () { + expect(name("int x", "last")).toBe("x"); + expect(name("const char *ptr", "last")).toBe("ptr"); + expect(name("String label", "last")).toBe("label"); + }); + }); + + describe("_parseSignature", function () { + const parse = DocCommentHints._parseSignature; + + it("parses a plain JS function (name-first, returns)", function () { + const sig = parse("function add(a, b) {", "first"); + expect(sig.params).toEqual(["a", "b"]); + expect(sig.isClass).toBe(false); + expect(sig.hasReturn).toBe(true); + }); + it("parses a typed TS function and keeps return", function () { + const sig = parse("function f(name: string, count: number = 1): boolean {", "first"); + expect(sig.params).toEqual(["name", "count"]); + expect(sig.hasReturn).toBe(true); + }); + it("treats an explicit void return as no @returns", function () { + expect(parse("function log(msg: string): void {", "first").hasReturn).toBe(false); + }); + it("skips self in a Python method", function () { + const sig = parse("def greet(self, name):", "first"); + expect(sig.params).toEqual(["name"]); + }); + it("parses a Java method type-first", function () { + const sig = parse("public int sum(int a, String b) {", "last"); + expect(sig.params).toEqual(["a", "b"]); + expect(sig.hasReturn).toBe(true); + }); + it("parses an arrow function", function () { + expect(parse("const mul = (a, b) => {", "first").params).toEqual(["a", "b"]); + }); + it("handles a rest parameter", function () { + expect(parse("function f(a, ...rest) {", "first").params).toEqual(["a", "rest"]); + }); + it("reports a class with no params/return", function () { + const sig = parse("export class Bar extends Base {", "first"); + expect(sig.isClass).toBe(true); + expect(sig.params).toEqual([]); + expect(sig.hasReturn).toBe(false); + }); + it("treats a constructor as returning nothing", function () { + const sig = parse("constructor(x, y) {", "first"); + expect(sig.params).toEqual(["x", "y"]); + expect(sig.hasReturn).toBe(false); + }); + it("handles an empty parameter list", function () { + expect(parse("function noop() {", "first").params).toEqual([]); + }); + it("flags isDeclaration only for real declarations (gates the partial /, /* triggers)", function () { + expect(parse("function add(a, b) {", "first").isDeclaration).toBe(true); + expect(parse("class Bar {", "first").isDeclaration).toBe(true); + expect(parse("const x = 5;", "first").isDeclaration).toBe(false); + expect(parse("return total;", "first").isDeclaration).toBe(false); + }); + }); + + describe("_buildSnippet", function () { + const build = DocCommentHints._buildSnippet; + + // Every generated line must be free of trailing whitespace (linters flag it - see the + // no-trailing-spaces report that prompted this). + function expectNoTrailingWhitespace(snippet) { + snippet.split("\n").forEach(function (line) { + expect(line).toBe(line.replace(/\s+$/, "")); + }); + } + + it("builds a JSDoc skeleton with type tabstops and no trailing whitespace", function () { + const snip = build("jsdoc", { params: ["a", "b"], isClass: false, hasReturn: true }, ""); + expect(snip.indexOf("/**")).toBe(0); + expect(snip).toContain("${1:"); // summary tabstop (default text is localized) + expect(snip).toContain("@param {${2:*}} a"); + expect(snip).toContain("@param {${3:*}} b"); + expect(snip).toContain("@returns {${4:*}}"); + expect(snip.trim().slice(-2)).toBe("*/"); + expectNoTrailingWhitespace(snip); + }); + it("omits @param/@returns for a class", function () { + const snip = build("jsdoc", { params: [], isClass: true, hasReturn: false }, ""); + expect(snip).toContain("${1:"); // summary tabstop only + expect(snip).not.toContain("@param"); + expect(snip).not.toContain("@returns"); + expectNoTrailingWhitespace(snip); + }); + it("indents continuation lines", function () { + const snip = build("jsdoc", { params: ["a"], isClass: false, hasReturn: false }, " "); + expect(snip).toContain("\n * @param"); // 4-space indent + " * " + }); + it("escapes $ in a PHP-style parameter name", function () { + const snip = build("jsdoc", { params: ["$user"], isClass: false, hasReturn: false }, ""); + expect(snip).toContain("\\$user"); + }); + it("builds a Python docstring with Args/Returns and no trailing whitespace", function () { + const snip = build("pydoc", { params: ["name"], isClass: false, hasReturn: true }, " "); + expect(snip.indexOf('"""')).toBe(0); + expect(snip).toContain("Args:"); + expect(snip).toContain("name: ${2:"); + expect(snip).toContain("Returns:"); + expectNoTrailingWhitespace(snip); + }); + }); + }); +}); diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index 097540389a..e0ae090100 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -1142,6 +1142,10 @@ define({ "CMD_OPEN_LINE_BELOW": "Open Line Below", "CMD_TOGGLE_CLOSE_BRACKETS": "Auto Close Braces", "CMD_SHOW_CODE_HINTS": "Show Code Hints", + "DOC_COMMENT_ADD_JSDOC": "Add JSDoc comment", + "DOC_COMMENT_ADD_DOCSTRING": "Add docstring", + "DOC_COMMENT_SUMMARY": "Description", + "DOC_COMMENT_DESC": "description", "CMD_BEAUTIFY_CODE": "Beautify Code", "CMD_BEAUTIFY_CODE_ON_SAVE": "Beautify Code After Save", "CMD_AUTO_RENAME_TAGS": "Auto Rename HTML Tags", diff --git a/src/styles/brackets.less b/src/styles/brackets.less index a40fd989e5..2b8e8945c3 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -2587,6 +2587,17 @@ span.brackets-hints-with-type-details { display: inline-block; } +// DocCommentHints "Add JSDoc comment" / "Add docstring" row: a muted monospace opener marker +// followed by the action label, so it reads as "insert a doc comment" rather than a cryptic token. +.doc-comment-hint .doc-comment-hint-marker { + font-family: 'SourceCodePro', 'SF Mono', Menlo, Consolas, monospace; + opacity: 0.6; + margin-right: 2px; +} +.doc-comment-hint .doc-comment-hint-label { + font-weight: 500; +} + .brackets-hints-type-details { color: #a3a3a3 !important; font-weight: 100; diff --git a/test/SpecRunner.js b/test/SpecRunner.js index 9a3bc6068e..a4d428986a 100644 --- a/test/SpecRunner.js +++ b/test/SpecRunner.js @@ -286,6 +286,8 @@ define(function (require, exports, module) { //load language features require("features/ParameterHintsManager"); require("features/JumpToDefManager"); + // Preload so DocCommentHints (and its unit spec) can synchronously brackets.getModule it. + require("editor/TabstopManager"); //node connector require("NodeConnector"); From a31f8a4089b16fe890073d2f3684502e96799b56 Mon Sep 17 00:00:00 2001 From: abose Date: Tue, 30 Jun 2026 21:15:33 +0530 Subject: [PATCH 2/5] fix(codehints): per-language doc-comment styles + Python docstring fixes The doc-comment hint applied JSDoc to every block-comment language - wrong for Java/C/C++ (Javadoc/Doxygen don't use {type} braces) and mislabeled ("Add JSDoc comment" in a .cpp file). Now each language gets its own convention and label: - jsdoc (JS/TS/JSX/TSX): @param {type} name / @returns {type} - phpdoc (PHP): @param type $name / @return type - tagdoc (Java/C/C++): @param name / @return (no {type} braces) - pydoc (Python): """ ... Args:/Returns: ... """ Rust is dropped (its doc comments are /// markdown, not @param). Python fixes: - triggers on the auto-closed `""` state (typing " auto-closes to "" with the caret between the quotes); matches the whole line and replaces it on insert so the stray quote is swallowed. - only emits a Returns: section when the def has an explicit `-> Type` annotation (an untyped function usually returns None - don't guess). Tests: unit:DocCommentHints now drives the real provider per language on a mock editor (hasHints -> getHints -> insertHint), and a new integration:DocCommentHints opens a file per language in a test window and asserts the code-hints popup actually appears. --- .../DocCommentHints/integration-tests.js | 80 +++++++++ .../default/DocCommentHints/main.js | 160 +++++++++++++----- .../default/DocCommentHints/unittests.js | 110 +++++++++++- src/nls/root/strings.js | 3 + 4 files changed, 311 insertions(+), 42 deletions(-) create mode 100644 src/extensions/default/DocCommentHints/integration-tests.js diff --git a/src/extensions/default/DocCommentHints/integration-tests.js b/src/extensions/default/DocCommentHints/integration-tests.js new file mode 100644 index 0000000000..3c7688fb6c --- /dev/null +++ b/src/extensions/default/DocCommentHints/integration-tests.js @@ -0,0 +1,80 @@ +/* + * GNU AGPL-3.0 License + * + * Copyright (c) 2021 - present core.ai . All rights reserved. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License + * for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://opensource.org/licenses/AGPL-3.0. + * + */ + +/*global describe, it, expect, beforeAll, afterAll, awaitsForDone, awaitsFor, jsPromise */ + +define(function (require, exports, module) { + const SpecRunnerUtils = brackets.getModule("spec/SpecRunnerUtils"); + + // Opener already present in each fixture; the test just parks the caret on it and asks for hints. + const LANGS = [ + { file: "a.js", line: 0, ch: 3, content: "/**\nfunction f(a, b) {\n}\n" }, + { file: "a.php", line: 1, ch: 3, content: " 0; + }).length > 0; + } + + LANGS.forEach(function (lang) { + it("shows the doc-comment hint popup for " + lang.file, async function () { + await awaitsForDone(SpecRunnerUtils.openProjectFiles([lang.file]), "open " + lang.file); + const editor = EditorManager.getActiveEditor(); + editor.setCursorPos(lang.line, lang.ch); + CommandManager.execute(Commands.SHOW_CODE_HINTS); + await awaitsFor(docCommentHintVisible, + "doc-comment hint popup for " + lang.file, 5000); + expect(docCommentHintVisible()).toBe(true); + // Dismiss the popup before the next file so sessions don't bleed across specs. + const target = $(".codehint-menu:visible")[0] || testWindow.document.body; + SpecRunnerUtils.simulateKeyEvent(27, "keydown", target); + }, 15000); + }); + }); +}); diff --git a/src/extensions/default/DocCommentHints/main.js b/src/extensions/default/DocCommentHints/main.js index b9731f08fe..5c2381c346 100644 --- a/src/extensions/default/DocCommentHints/main.js +++ b/src/extensions/default/DocCommentHints/main.js @@ -47,34 +47,55 @@ define(function (require, exports, module) { // context, where those providers have nothing useful to offer anyway. const PROVIDER_PRIORITY = 100; + // Doc-comment styles. The `/** */` syntax is shared, but the convention differs: + // jsdoc - @param {type} name / @returns {type} (JS/TS family) + // phpdoc - @param type $name / @return type (PHP: type before the $name) + // tagdoc - @param name / @return (Javadoc & Doxygen: no {type} braces) + // pydoc - """ ... Args:/Returns: ... """ (Python docstring) const STYLE_JSDOC = "jsdoc", + STYLE_PHPDOC = "phpdoc", + STYLE_TAGDOC = "tagdoc", STYLE_PYDOC = "pydoc"; // Per-language config: the doc-comment style, where a parameter's NAME sits in a declaration - // ("first" -> `name: Type` / `name` / `$name`; "last" -> `Type name`, the C/Java family), and - // which opener triggers the hint. + // ("first" -> `name: Type` / `name` / `$name`; "last" -> `Type name`, the C/Java family), which + // opener triggers the hint, and the (localized) label naming that language's convention. Rust is + // intentionally absent: its doc comments are `///` markdown (# Arguments), not `/** @param */`. const LANGUAGES = { - javascript: { style: STYLE_JSDOC, params: "first", opener: "block" }, - typescript: { style: STYLE_JSDOC, params: "first", opener: "block" }, - jsx: { style: STYLE_JSDOC, params: "first", opener: "block" }, - tsx: { style: STYLE_JSDOC, params: "first", opener: "block" }, - php: { style: STYLE_JSDOC, params: "first", opener: "block" }, - rust: { style: STYLE_JSDOC, params: "first", opener: "block" }, - java: { style: STYLE_JSDOC, params: "last", opener: "block" }, - c: { style: STYLE_JSDOC, params: "last", opener: "block" }, - cpp: { style: STYLE_JSDOC, params: "last", opener: "block" }, - python: { style: STYLE_PYDOC, params: "first", opener: "pydoc" } + javascript: { style: STYLE_JSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, + typescript: { style: STYLE_JSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, + jsx: { style: STYLE_JSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, + tsx: { style: STYLE_JSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, + php: { style: STYLE_PHPDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_PHPDOC" }, + java: { style: STYLE_TAGDOC, params: "last", opener: "block", label: "DOC_COMMENT_ADD_JAVADOC" }, + c: { style: STYLE_TAGDOC, params: "last", opener: "block", label: "DOC_COMMENT_ADD_DOXYGEN" }, + cpp: { style: STYLE_TAGDOC, params: "last", opener: "block", label: "DOC_COMMENT_ADD_DOXYGEN" }, + python: { style: STYLE_PYDOC, params: "first", opener: "pydoc", label: "DOC_COMMENT_ADD_DOCSTRING" } }; - // Opener: `full` is the canonical opener (offered unconditionally); `partial` also matches the - // half-typed forms (`/`, `/*`) so the hint can appear as soon as the user starts the comment - - // but a partial match is only offered when a documentable declaration sits next to it (see - // _openerContext), so a stray `/` on a line never spams the list. `chars` are the keystrokes that - // may trigger an implicit request; `dir` is where the documented declaration lives relative to the - // opener (below it for block comments, above it for a Python docstring). + // Opener config: + // full - the canonical opener; offered unconditionally unless `alwaysGate`. + // partial - also matches half-typed forms (`/`, `/*`, or `"`/`""` once auto-close kicks in) so + // the hint appears as the user starts the comment; only offered with an adjacent + // declaration (see _openerContext), so a stray `/` or empty string never spams it. + // chars - keystrokes that may trigger an implicit (type-as-you-go) request. + // dir - where the documented declaration lives: "below" (block comment above a function) + // or "above" (a Python docstring sits under its `def`/`class`). + // fullLine - match the trimmed full line, not just text-before-cursor. Python needs this because + // typing `"` auto-closes to `""` with the caret between the quotes. + // alwaysGate- require a declaration even for the full opener (Python: a lone `"""`/`""` with no + // `def`/`class` above is just an empty string literal, not a docstring). + // replaceLine- on insert, replace the whole line (to swallow auto-closed quotes) vs only up to + // the caret. const OPENERS = { - block: { full: /^\s*\/\*\*$/, partial: /^\s*\/\*{0,2}$/, chars: "/*", dir: "below" }, - pydoc: { full: /^\s*"""$/, partial: /^\s*"""$/, chars: "\"", dir: "above" } + block: { + full: /^\s*\/\*\*$/, partial: /^\s*\/\*{0,2}$/, chars: "/*", dir: "below", + fullLine: false, alwaysGate: false, replaceLine: false + }, + pydoc: { + full: /^\s*"{3,6}$/, partial: /^\s*"{1,6}$/, chars: "\"", dir: "above", + fullLine: true, alwaysGate: true, replaceLine: true + } }; function _configFor(editor) { @@ -204,10 +225,17 @@ define(function (require, exports, module) { } }); // Return: a constructor returns nothing; an explicit `void`/`None`/`-> ()` return type is void. + // `hasReturnType` is stricter - an EXPLICIT non-None return annotation (`-> Type`). Python uses + // it (no annotation -> we can't tell, and most untyped Python returns None, so we omit Returns + // rather than guess); the C-family/JS keep the looser `hasReturn` (their signature implies it). const after = close === -1 ? "" : declText.slice(close + 1); const isCtor = /\b(constructor|__init__)\b/.test(declText) || /\bvoid\s+\w+\s*\(/.test(declText); const isVoid = /:\s*void\b/.test(after) || /->\s*(None|\(\s*\))/.test(after) || isCtor; - return { params: params, isClass: false, hasReturn: !isVoid, isDeclaration: true }; + const hasReturnType = /->\s*(?!None\b)[A-Za-z_]/.test(after); + return { + params: params, isClass: false, hasReturn: !isVoid, + hasReturnType: hasReturnType, isDeclaration: true + }; } // ----- snippet building ------------------------------------------------------------------ @@ -235,6 +263,40 @@ define(function (require, exports, module) { return out.join("\n"); } + // PHPDoc: `@param type $name`, `@return type`. The type sits before the (kept) $name, no braces. + function _buildPhpDoc(sig, indent) { + const star = indent + " * "; + const out = ["/**", star + "${1:" + _escDesc(Strings.DOC_COMMENT_SUMMARY) + "}"]; + let stop = 2; + if (sig && !sig.isClass) { + sig.params.forEach(function (p) { + out.push(star + "@param ${" + (stop++) + ":mixed} " + _esc(p)); + }); + if (sig.hasReturn) { + out.push(star + "@return ${" + (stop++) + ":mixed}"); + } + } + out.push(indent + " */"); + return out.join("\n"); + } + + // Javadoc / Doxygen: `@param name`, `@return` - no {type} braces (types come from the signature), + // and the singular `@return`. The only tabstop is the summary; param names are pre-filled. + function _buildTagDoc(sig, indent) { + const star = indent + " * "; + const out = ["/**", star + "${1:" + _escDesc(Strings.DOC_COMMENT_SUMMARY) + "}"]; + if (sig && !sig.isClass) { + sig.params.forEach(function (p) { + out.push(star + "@param " + _esc(p)); + }); + if (sig.hasReturn) { + out.push(star + "@return"); + } + } + out.push(indent + " */"); + return out.join("\n"); + } + function _buildPyDoc(sig, indent) { const out = ['"""${1:' + _escDesc(Strings.DOC_COMMENT_SUMMARY) + "}"]; let stop = 2; @@ -247,7 +309,9 @@ define(function (require, exports, module) { out.push(indent + " " + _esc(p) + ": ${" + (stop++) + ":" + desc + "}"); }); } - if (sig && sig.hasReturn) { + // Only document a return when the signature explicitly annotates one (`-> Type`); an untyped + // Python function usually returns None, so we don't guess a Returns section. + if (sig && sig.hasReturnType) { out.push(""); out.push(indent + "Returns:"); out.push(indent + " ${" + (stop++) + ":" + desc + "}"); @@ -262,7 +326,12 @@ define(function (require, exports, module) { } function _buildSnippet(style, sig, indent) { - return style === STYLE_PYDOC ? _buildPyDoc(sig, indent) : _buildJsDoc(sig, indent); + switch (style) { + case STYLE_PYDOC: return _buildPyDoc(sig, indent); + case STYLE_PHPDOC: return _buildPhpDoc(sig, indent); + case STYLE_TAGDOC: return _buildTagDoc(sig, indent); + default: return _buildJsDoc(sig, indent); + } } // ----- the hint provider ----------------------------------------------------------------- @@ -281,15 +350,21 @@ define(function (require, exports, module) { return null; } const cursor = editor.getCursorPos(); - const before = editor.document.getLine(cursor.line).slice(0, cursor.ch); - if (opener.full.test(before)) { + const lineText = editor.document.getLine(cursor.line); + // Python matches the whole (trimmed) line so auto-closed quotes (caret inside `""`) still count; + // block comments match only what's left of the caret. + const text = opener.fullLine ? lineText.trim() : lineText.slice(0, cursor.ch); + if (!opener.partial.test(text)) { + return null; + } + // A full opener is offered as-is, unless this language always requires a declaration nearby + // (Python, to tell a docstring apart from a bare empty-string literal). + if (opener.full.test(text) && !opener.alwaysGate) { return { cfg: cfg }; } - if (opener.partial.test(before)) { - const sig = _parseSignature(_declarationFor(editor, cursor.line, opener.dir), cfg.params); - if (sig && sig.isDeclaration) { - return { cfg: cfg }; - } + const sig = _parseSignature(_declarationFor(editor, cursor.line, opener.dir), cfg.params); + if (sig && sig.isDeclaration) { + return { cfg: cfg }; } return null; } @@ -299,7 +374,7 @@ define(function (require, exports, module) { DocCommentHintProvider.prototype.hasHints = function (editor, implicitChar) { this.editor = editor; const ctx = _openerContext(editor, implicitChar); - this._style = ctx && ctx.cfg.style; + this._cfg = ctx && ctx.cfg; return !!ctx; }; @@ -309,15 +384,15 @@ define(function (require, exports, module) { if (!this.editor || !_openerContext(this.editor, null)) { return null; } - // A clear, language-aware action label - not a cryptic "/**/". The leading marker echoes the - // doc-comment syntax so it reads as "insert a doc comment here". - const isPy = this._style === STYLE_PYDOC; - const label = isPy ? Strings.DOC_COMMENT_ADD_DOCSTRING : Strings.DOC_COMMENT_ADD_JSDOC; + // A clear, language-aware action label (JSDoc / Javadoc / Doxygen / PHPDoc / docstring) - not a + // cryptic "/**/". The leading marker echoes the doc-comment syntax for that language. + const cfg = this._cfg; + const marker = OPENERS[cfg.opener].dir === "above" ? '"""' : "/**"; const $hint = $("") .addClass("doc-comment-hint") .data("docComment", true) - .append($("").addClass("doc-comment-hint-marker").text(isPy ? '"""' : "/**")) - .append($("").addClass("doc-comment-hint-label").text(" " + label)); + .append($("").addClass("doc-comment-hint-marker").text(marker)) + .append($("").addClass("doc-comment-hint-label").text(" " + Strings[cfg.label])); return { hints: [$hint], match: null, selectInitial: true, handleWideResults: true }; }; @@ -331,13 +406,16 @@ define(function (require, exports, module) { const line = editor.document.getLine(cursor.line); const indent = (line.match(/^\s*/) || [""])[0]; - const decl = _declarationFor(editor, cursor.line, OPENERS[cfg.opener].dir); + const opener = OPENERS[cfg.opener]; + const decl = _declarationFor(editor, cursor.line, opener.dir); const sig = _parseSignature(decl, cfg.params); const snippet = _buildSnippet(cfg.style, sig, indent); - // Replace the opener the user typed (from the start of `/**` / `"""` up to the caret). + // Replace the opener the user typed: from the start of `/**` / `"""`. For Python we replace the + // whole line (ch = line length) to swallow the quote that auto-close added after the caret; + // for block comments we stop at the caret (anything the user typed after is left alone). const startPos = { line: cursor.line, ch: indent.length }; - const endPos = { line: cursor.line, ch: cursor.ch }; + const endPos = { line: cursor.line, ch: opener.replaceLine ? line.length : cursor.ch }; TabstopManager.insertSnippet(editor, snippet, startPos, endPos); return false; }; @@ -352,4 +430,6 @@ define(function (require, exports, module) { exports._buildSnippet = _buildSnippet; exports._splitParams = _splitParams; exports._paramName = _paramName; + exports._Provider = DocCommentHintProvider; + exports._LANGUAGES = LANGUAGES; }); diff --git a/src/extensions/default/DocCommentHints/unittests.js b/src/extensions/default/DocCommentHints/unittests.js index dabd824566..e449b0485a 100644 --- a/src/extensions/default/DocCommentHints/unittests.js +++ b/src/extensions/default/DocCommentHints/unittests.js @@ -18,7 +18,7 @@ * */ -/*global describe, it, expect*/ +/*global describe, it, expect, afterEach*/ define(function (require, exports, module) { const DocCommentHints = require("./main"); @@ -149,14 +149,120 @@ define(function (require, exports, module) { const snip = build("jsdoc", { params: ["$user"], isClass: false, hasReturn: false }, ""); expect(snip).toContain("\\$user"); }); + it("builds a PHPDoc skeleton (type before the $name, @return, no braces)", function () { + const snip = build("phpdoc", { params: ["$a", "$b"], isClass: false, hasReturn: true }, ""); + expect(snip).toContain("@param ${2:mixed} \\$a"); + expect(snip).toContain("@param ${3:mixed} \\$b"); + expect(snip).toContain("@return ${4:mixed}"); + expect(snip).not.toContain("@param {"); + expectNoTrailingWhitespace(snip); + }); + it("builds a Javadoc/Doxygen skeleton (no {type} braces, singular @return)", function () { + const snip = build("tagdoc", { params: ["a", "b"], isClass: false, hasReturn: true }, ""); + expect(snip).toContain("@param a"); + expect(snip).toContain("@param b"); + expect(snip).toContain("@return"); + expect(snip).not.toContain("@returns"); + expect(snip).not.toContain("@param {"); + expectNoTrailingWhitespace(snip); + }); it("builds a Python docstring with Args/Returns and no trailing whitespace", function () { - const snip = build("pydoc", { params: ["name"], isClass: false, hasReturn: true }, " "); + const snip = build("pydoc", { params: ["name"], isClass: false, hasReturnType: true }, " "); expect(snip.indexOf('"""')).toBe(0); expect(snip).toContain("Args:"); expect(snip).toContain("name: ${2:"); expect(snip).toContain("Returns:"); + expect(snip).not.toContain("@param"); expectNoTrailingWhitespace(snip); }); + it("Python omits Returns without an explicit return annotation", function () { + const snip = build("pydoc", { params: ["name"], isClass: false, hasReturnType: false }, " "); + expect(snip).toContain("Args:"); + expect(snip).not.toContain("Returns:"); + }); + }); + + // Drives the REAL provider (hasHints -> getHints -> insertHint) on a real editor for each + // language: proves the hint fires for that language, shows the right label, and inserts that + // language's doc-comment convention. + describe("provider, per language", function () { + const SpecRunnerUtils = brackets.getModule("spec/SpecRunnerUtils"); + const Strings = brackets.getModule("strings"); + const TabstopManager = brackets.getModule("editor/TabstopManager"); + let mockDoc = null; + + afterEach(function () { + if (TabstopManager.hasActiveSession && TabstopManager.hasActiveSession()) { + TabstopManager.endSession(); + } + if (mockDoc) { + SpecRunnerUtils.destroyMockEditor(mockDoc); + mockDoc = null; + } + }); + + function run(langId, content, line, ch) { + const mock = SpecRunnerUtils.createMockEditor(content, langId); + mockDoc = mock.doc; + mock.editor.setCursorPos(line, ch); + const provider = new DocCommentHints._Provider(); + const has = provider.hasHints(mock.editor, null); + const hints = has ? provider.getHints() : null; + const label = hints && hints.hints[0].text(); + if (has) { + provider.insertHint(); + } + return { has: has, label: label, text: mock.doc.getText() }; + } + + const CASES = [ + { id: "javascript", content: "/**\nfunction f(a, b) {\n}\n", line: 0, ch: 3, + label: "DOC_COMMENT_ADD_JSDOC", has: ["@param {*} a", "@param {*} b", "@returns {*}"], absent: [] }, + { id: "typescript", content: "/**\nfunction f(a) {\n}\n", line: 0, ch: 3, + label: "DOC_COMMENT_ADD_JSDOC", has: ["@param {*} a"], absent: [] }, + { id: "php", content: " int:\n """\n', line: 1, ch: 7, + label: "DOC_COMMENT_ADD_DOCSTRING", has: ["Args:", "a:", "Returns:"], absent: ["@param", "{*}"] } + ]; + + CASES.forEach(function (tc) { + it("offers and inserts the right doc comment for " + tc.id, function () { + const r = run(tc.id, tc.content, tc.line, tc.ch); + expect(r.has).toBe(true); + expect(r.label).toContain(Strings[tc.label]); + tc.has.forEach(function (frag) { expect(r.text).toContain(frag); }); + tc.absent.forEach(function (frag) { expect(r.text).not.toContain(frag); }); + }); + }); + + it("does NOT fire for an unsupported language (css)", function () { + expect(run("css", "/**\n.x { color: red; }\n", 0, 3).has).toBe(false); + }); + + it("Python adds no Returns when the def has no return annotation", function () { + const r = run("python", 'def f(a):\n """\n', 1, 7); + expect(r.has).toBe(true); + expect(r.text).toContain("Args:"); + expect(r.text).not.toContain("Returns:"); + }); + + it("Python fires on the auto-closed \"\" state (caret between the quotes)", function () { + // typing " auto-closes to "" with the caret inside; the hint must still fire. + const r = run("python", 'def f(a):\n ""\n', 1, 5); + expect(r.has).toBe(true); + expect(r.text).toContain('"""'); + }); }); }); + + // Real-window integration: confirms the code-hints POPUP actually appears for these languages. + require("./integration-tests"); }); diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index e0ae090100..0197e24a47 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -1143,6 +1143,9 @@ define({ "CMD_TOGGLE_CLOSE_BRACKETS": "Auto Close Braces", "CMD_SHOW_CODE_HINTS": "Show Code Hints", "DOC_COMMENT_ADD_JSDOC": "Add JSDoc comment", + "DOC_COMMENT_ADD_PHPDOC": "Add PHPDoc comment", + "DOC_COMMENT_ADD_JAVADOC": "Add Javadoc comment", + "DOC_COMMENT_ADD_DOXYGEN": "Add Doxygen comment", "DOC_COMMENT_ADD_DOCSTRING": "Add docstring", "DOC_COMMENT_SUMMARY": "Description", "DOC_COMMENT_DESC": "description", From ca7cedc61dad8aadd68562503daae1e57ee0463f Mon Sep 17 00:00:00 2001 From: abose Date: Tue, 30 Jun 2026 21:23:57 +0530 Subject: [PATCH 3/5] build: register DocCommentHints in nonMinifyExtensions The release build validates that every default extension is listed in either minifyableExtensions or nonMinifyExtensions; the new DocCommentHints extension was missing, failing makeConcatExtensions. --- gulpfile.js/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gulpfile.js/index.js b/gulpfile.js/index.js index 8d1b186de1..3cf6f3427e 100644 --- a/gulpfile.js/index.js +++ b/gulpfile.js/index.js @@ -912,9 +912,9 @@ const minifyableExtensions = ["CloseOthers", "CodeFolding", "DebugCommands", "Gi "HealthData", "JavaScriptCodeHints", "JavaScriptRefactoring", "QuickView", "TypeScriptSupport"]; // extensions that nned not be minified either coz they are single file extensions or some other reason. const nonMinifyExtensions = ["CSSAtRuleCodeHints", "CSSCodeHints", - "CSSPseudoSelectorHints", "DarkTheme", "HandlebarsSupport", "HTMLCodeHints", "HtmlEntityCodeHints", - "InlineColorEditor", "InlineTimingFunctionEditor", "JavaScriptQuickEdit", "JSLint", - "LightTheme", "MDNDocs", "Phoenix-prettier", "PrefsCodeHints", "SVGCodeHints", "UrlCodeHints" + "CSSPseudoSelectorHints", "DarkTheme", "DocCommentHints", "HandlebarsSupport", "HTMLCodeHints", + "HtmlEntityCodeHints", "InlineColorEditor", "InlineTimingFunctionEditor", "JavaScriptQuickEdit", + "JSLint", "LightTheme", "MDNDocs", "Phoenix-prettier", "PrefsCodeHints", "SVGCodeHints", "UrlCodeHints" ]; async function makeConcatExtensions() { let content = JSON.parse(fs.readFileSync(`src/extensions/default/DefaultExtensions.json`, From 345d93400b7c789b53e5fbff3f5fb460271be24d Mon Sep 17 00:00:00 2001 From: abose Date: Tue, 30 Jun 2026 21:43:41 +0530 Subject: [PATCH 4/5] feat(codehints): fill real @param/@returns types from the TS/JS signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JSDoc generation used a constant {*}. For TypeScript (and JSDoc-typed JS) the types are already in the signature being documented, so we now parse them in - @param {number} a, @returns {Promise} - synchronously, no language server. Untyped JS stays {*}. The parser is hardened for complex types via a single top-level scanner that tracks () [] {} <> nesting, treats `>` after `=` as the arrow (not a generic close), and skips string/template contents. This makes param splitting and name/type extraction reliable for generics, function types, object/tuple types, unions, conditionals, optionals (`?` -> [name]), defaults and rest. Any annotation that doesn't balance falls back to {*}, so a broken `@param {…}` is never emitted. _parseSignature now returns params as {name, type, optional} plus returnType; the jsdoc builder renders the type as the tabstop default (editable), the other styles are unchanged (read p.name). Scope: jsdoc (JS/TS/JSX/TSX) only, per request. Tests: new _parseParam/_validType suites cover the complex-type cases and the unbalanced-fallback; _parseSignature asserts extracted param/return types; the TS provider case now asserts real {number}/{string}/{boolean}. --- .../default/DocCommentHints/main.js | 222 ++++++++++++++---- .../default/DocCommentHints/unittests.js | 132 ++++++++--- 2 files changed, 267 insertions(+), 87 deletions(-) diff --git a/src/extensions/default/DocCommentHints/main.js b/src/extensions/default/DocCommentHints/main.js index 5c2381c346..2df6905fa2 100644 --- a/src/extensions/default/DocCommentHints/main.js +++ b/src/extensions/default/DocCommentHints/main.js @@ -104,10 +104,78 @@ define(function (require, exports, module) { // ----- signature parsing ----------------------------------------------------------------- - function _delta(ch) { - if (ch === "(" || ch === "[" || ch === "{" || ch === "<") { return 1; } - if (ch === ")" || ch === "]" || ch === "}" || ch === ">") { return -1; } - return 0; + // The reliability core. Walk `s` left to right calling visit(char, index, atTopLevel) for each + // non-bracket/non-quote char, where atTopLevel means depth 0 AND outside any string. Tracks + // () [] {} <> nesting - with the key subtlety that a `>` right after `=` is the arrow `=>`, not a + // generic close - and skips the contents of '…' "…" `…` literals (respecting \ escapes). This lets + // us split params and find the name/type separator without tripping on commas/colons inside + // generics, function types, object types, tuples or strings. + function _eachTopLevel(s, visit) { + let depth = 0, quote = null; + for (let i = 0; i < s.length; i++) { + const c = s[i]; + if (quote) { + if (c === "\\") { i++; } else if (c === quote) { quote = null; } + continue; + } + if (c === '"' || c === "'" || c === "`") { quote = c; continue; } + const open = (c === "(" || c === "[" || c === "{" || c === "<"); + const close = (c === ")" || c === "]" || c === "}" || (c === ">" && s[i - 1] !== "=")); + if (!open && !close) { + visit(c, i, depth === 0); + } + if (open) { depth++; } else if (close) { depth--; } + } + } + + // Split `s` on `sep` only where it sits at the top level (depth 0, outside strings). + function _splitTopLevel(s, sep) { + const out = []; + let start = 0; + _eachTopLevel(s, function (c, i, top) { + if (top && c === sep) { + out.push(s.slice(start, i)); + start = i + 1; + } + }); + out.push(s.slice(start)); + return out; + } + + // Index of the first char satisfying pred at the top level, or -1. + function _indexOfTopLevel(s, pred) { + let found = -1; + _eachTopLevel(s, function (c, i, top) { + if (found === -1 && top && pred(c, i)) { found = i; } + }); + return found; + } + + // A type substring is safe to emit only if its brackets/generics/strings balance; anything else + // falls back to `*` so we never write a broken `@param {…}`. + function _validType(t) { + t = t.trim(); + if (!t) { + return false; + } + let depth = 0, quote = null; + for (let i = 0; i < t.length; i++) { + const c = t[i]; + if (quote) { + if (c === "\\") { i++; } else if (c === quote) { quote = null; } + continue; + } + if (c === '"' || c === "'" || c === "`") { quote = c; continue; } + if (c === "(" || c === "[" || c === "{" || c === "<") { + depth++; + } else if (c === ")" || c === "]" || c === "}" || (c === ">" && t[i - 1] !== "=")) { + depth--; + } + if (depth < 0) { + return false; + } + } + return depth === 0 && quote === null; } // The declaration the doc comment documents, joining wrapped lines until its parameter parens @@ -143,57 +211,104 @@ define(function (require, exports, module) { return depth; } - // Split a parameter list on top-level commas, ignoring commas nested in (), [], {}, <>. + // Split a parameter list on top-level commas, ignoring commas nested in (), [], {}, <>, strings. function _splitParams(s) { - const out = []; - let depth = 0, start = 0; - for (let i = 0; i < s.length; i++) { - depth += _delta(s[i]); - if (s[i] === "," && depth === 0) { - out.push(s.slice(start, i)); - start = i + 1; - } - } - if (s.slice(start).trim() !== "") { - out.push(s.slice(start)); - } - return out; + return _splitTopLevel(s, ",").filter(function (t) { + return t.trim() !== ""; + }); } const IDENT = /[A-Za-z_][\w]*/g; const PARAMS_TO_SKIP = { self: true, cls: true, this: true, void: true }; - // Extract a clean parameter name from one parameter token per the language's name convention. - function _paramName(token, convention) { + // A top-level `=` that begins a default value - not part of `=>` `==` `<=` `>=` `!=`. + function _isDefaultEq(t) { + return function (c, i) { + return c === "=" && t[i + 1] !== ">" && t[i + 1] !== "=" && + t[i - 1] !== "=" && t[i - 1] !== "<" && t[i - 1] !== ">" && t[i - 1] !== "!"; + }; + } + + /** + * Parse one parameter token into its name, declared type and whether it is optional. + * For "first" languages (JS/TS/PHP) the type is the part after the top-level `:`; for "last" + * languages (C/Java) there is no `:`-type to surface (tagdoc/phpdoc don't render {type}), so type + * stays null. An unparseable/unbalanced type also stays null (the builder falls back to `*`). + * @return {?{name: string, type: ?string, optional: boolean}} + */ + function _parseParam(token, convention) { let t = token.trim(); if (!t) { return null; } - // Drop a default value (top-level '='). - let depth = 0; - for (let i = 0; i < t.length; i++) { - depth += _delta(t[i]); - if (t[i] === "=" && depth === 0) { - t = t.slice(0, i); - break; - } + t = t.replace(/^\.\.\.\s*/, ""); // rest/spread + const eq = _indexOfTopLevel(t, _isDefaultEq(t)); + if (eq !== -1) { + t = t.slice(0, eq); } - t = t.replace(/\.\.\./g, " ").trim(); // rest/spread + const colon = _indexOfTopLevel(t, function (c) { return c === ":"; }); + let namePart = (colon === -1 ? t : t.slice(0, colon)).trim(); + let typePart = colon === -1 ? null : t.slice(colon + 1).trim(); + let optional = false; + if (namePart.endsWith("?")) { + optional = true; + namePart = namePart.slice(0, -1).trim(); + } + let name; if (convention === "first") { - // `name`, `name: Type`, `$name` (PHP). Keep a leading $ if present. - const m = t.match(/^[*&\s]*(\$?[A-Za-z_][\w]*)/); - return m ? m[1] : null; + const m = namePart.match(/^[*&\s]*(\$?[A-Za-z_][\w]*)/); // keep a leading $ for PHP + name = m ? m[1] : null; + } else { + const ids = namePart.match(IDENT); // C/Java `Type name` -> trailing identifier + name = ids && ids.length ? ids[ids.length - 1] : null; + typePart = null; + } + if (!name) { + return null; + } + return { + name: name, + type: (typePart && _validType(typePart)) ? typePart.trim() : null, + optional: optional + }; + } + + // The declared return type for the jsdoc builder: the TS `: Type` (or `-> Type`) after the param + // list, read up to the body `{`, an arrow `=>`, a `;`, or end. Null when absent/unbalanced. + function _returnType(after) { + const m = after.match(/^\s*(?::|->)\s*/); + if (!m) { + return null; + } + const s = after.slice(m[0].length); + let depth = 0, quote = null, end = s.length; + for (let i = 0; i < s.length; i++) { + const c = s[i]; + if (quote) { + if (c === "\\") { i++; } else if (c === quote) { quote = null; } + continue; + } + if (c === '"' || c === "'" || c === "`") { quote = c; continue; } + if (depth === 0 && (c === "{" || c === ";" || (c === "=" && s[i + 1] === ">"))) { + end = i; + break; + } + if (c === "(" || c === "[" || c === "{" || c === "<") { + depth++; + } else if (c === ")" || c === "]" || c === "}" || (c === ">" && s[i - 1] !== "=")) { + depth--; + } } - // "last": C/Java `Type name`, `const char *name` -> the trailing identifier is the name. - const ids = t.match(IDENT); - return ids && ids.length ? ids[ids.length - 1] : null; + const type = s.slice(0, end).trim(); + return _validType(type) ? type : null; } /** * Parse a declaration line into a documentable signature. `isDeclaration` is true only when the * text actually looks like something to document (a function/method - has a parameter list - or a * class), which is what gates the half-typed `/` `/*` triggers. - * @return {?{params: string[], isClass: boolean, hasReturn: boolean, isDeclaration: boolean}} + * @return {?{params: Array<{name,type,optional}>, returnType: ?string, isClass: boolean, + * hasReturn: boolean, hasReturnType: boolean, isDeclaration: boolean}} */ function _parseSignature(declText, convention) { if (!declText) { @@ -202,10 +317,12 @@ define(function (require, exports, module) { const open = declText.indexOf("("); const classMatch = /\b(class|interface|struct|enum|trait)\b/.exec(declText); if (classMatch && (open === -1 || classMatch.index < open)) { - return { params: [], isClass: true, hasReturn: false, isDeclaration: true }; + return { params: [], returnType: null, isClass: true, hasReturn: false, + hasReturnType: false, isDeclaration: true }; } if (open === -1) { - return { params: [], isClass: false, hasReturn: false, isDeclaration: false }; + return { params: [], returnType: null, isClass: false, hasReturn: false, + hasReturnType: false, isDeclaration: false }; } let depth = 0, close = -1; for (let i = open; i < declText.length; i++) { @@ -219,9 +336,9 @@ define(function (require, exports, module) { const inner = close === -1 ? declText.slice(open + 1) : declText.slice(open + 1, close); const params = []; _splitParams(inner).forEach(function (tok) { - const name = _paramName(tok, convention); - if (name && !PARAMS_TO_SKIP[name]) { - params.push(name); + const p = _parseParam(tok, convention); + if (p && !PARAMS_TO_SKIP[p.name]) { + params.push(p); } }); // Return: a constructor returns nothing; an explicit `void`/`None`/`-> ()` return type is void. @@ -233,7 +350,7 @@ define(function (require, exports, module) { const isVoid = /:\s*void\b/.test(after) || /->\s*(None|\(\s*\))/.test(after) || isCtor; const hasReturnType = /->\s*(?!None\b)[A-Za-z_]/.test(after); return { - params: params, isClass: false, hasReturn: !isVoid, + params: params, returnType: _returnType(after), isClass: false, hasReturn: !isVoid, hasReturnType: hasReturnType, isDeclaration: true }; } @@ -250,13 +367,17 @@ define(function (require, exports, module) { const out = ["/**", star + "${1:" + _escDesc(Strings.DOC_COMMENT_SUMMARY) + "}"]; let stop = 2; if (sig && !sig.isClass) { - // Tabstop on the {type} only; no trailing description stub (an empty tabstop would leave a - // trailing space that linters flag). Descriptions are added inline by the user. + // The {type} is a tabstop pre-filled with the real type from the signature (TS) or `*` when + // untyped (JS) - selected on Tab so the user can refine it. Optional params render [name] + // (standard JSDoc). No trailing description stub (an empty tabstop leaves a flagged space). sig.params.forEach(function (p) { - out.push(star + "@param {${" + (stop++) + ":*}} " + _esc(p)); + const type = p.type ? _esc(p.type) : "*"; + const nameOut = p.optional ? "[" + _esc(p.name) + "]" : _esc(p.name); + out.push(star + "@param {${" + (stop++) + ":" + type + "}} " + nameOut); }); if (sig.hasReturn) { - out.push(star + "@returns {${" + (stop++) + ":*}}"); + const rtype = sig.returnType ? _esc(sig.returnType) : "*"; + out.push(star + "@returns {${" + (stop++) + ":" + rtype + "}}"); } } out.push(indent + " */"); @@ -270,7 +391,7 @@ define(function (require, exports, module) { let stop = 2; if (sig && !sig.isClass) { sig.params.forEach(function (p) { - out.push(star + "@param ${" + (stop++) + ":mixed} " + _esc(p)); + out.push(star + "@param ${" + (stop++) + ":mixed} " + _esc(p.name)); }); if (sig.hasReturn) { out.push(star + "@return ${" + (stop++) + ":mixed}"); @@ -287,7 +408,7 @@ define(function (require, exports, module) { const out = ["/**", star + "${1:" + _escDesc(Strings.DOC_COMMENT_SUMMARY) + "}"]; if (sig && !sig.isClass) { sig.params.forEach(function (p) { - out.push(star + "@param " + _esc(p)); + out.push(star + "@param " + _esc(p.name)); }); if (sig.hasReturn) { out.push(star + "@return"); @@ -306,7 +427,7 @@ define(function (require, exports, module) { out.push(indent + "Args:"); // Non-empty placeholder (selected on tab) so the line carries no trailing whitespace. sig.params.forEach(function (p) { - out.push(indent + " " + _esc(p) + ": ${" + (stop++) + ":" + desc + "}"); + out.push(indent + " " + _esc(p.name) + ": ${" + (stop++) + ":" + desc + "}"); }); } // Only document a return when the signature explicitly annotates one (`-> Type`); an untyped @@ -429,7 +550,8 @@ define(function (require, exports, module) { exports._parseSignature = _parseSignature; exports._buildSnippet = _buildSnippet; exports._splitParams = _splitParams; - exports._paramName = _paramName; + exports._parseParam = _parseParam; + exports._validType = _validType; exports._Provider = DocCommentHintProvider; exports._LANGUAGES = LANGUAGES; }); diff --git a/src/extensions/default/DocCommentHints/unittests.js b/src/extensions/default/DocCommentHints/unittests.js index e449b0485a..30b854e7e8 100644 --- a/src/extensions/default/DocCommentHints/unittests.js +++ b/src/extensions/default/DocCommentHints/unittests.js @@ -42,68 +42,107 @@ define(function (require, exports, module) { }); }); - describe("_paramName - per-convention name extraction", function () { - const name = DocCommentHints._paramName; - it("name-first: plain, typed, default, PHP, rest", function () { - expect(name("name", "first")).toBe("name"); - expect(name("name: string", "first")).toBe("name"); - expect(name("count = 5", "first")).toBe("count"); - expect(name("count: number = 1", "first")).toBe("count"); - expect(name("$user", "first")).toBe("$user"); - expect(name("...rest", "first")).toBe("rest"); - }); - it("type-first (C/Java): trailing identifier is the name", function () { - expect(name("int x", "last")).toBe("x"); - expect(name("const char *ptr", "last")).toBe("ptr"); - expect(name("String label", "last")).toBe("label"); + describe("_parseParam - name, type and optional", function () { + const parse = DocCommentHints._parseParam; + function nt(token, conv) { + return parse(token, conv || "first"); + } + it("name-first: plain / typed / default / PHP / rest", function () { + expect(nt("name")).toEqual({ name: "name", type: null, optional: false }); + expect(nt("name: string")).toEqual({ name: "name", type: "string", optional: false }); + expect(nt("count = 5")).toEqual({ name: "count", type: null, optional: false }); + expect(nt("count: number = 1")).toEqual({ name: "count", type: "number", optional: false }); + expect(nt("$user")).toEqual({ name: "$user", type: null, optional: false }); + expect(nt("...rest: number[]")).toEqual({ name: "rest", type: "number[]", optional: false }); + }); + it("marks optional params (`?`)", function () { + expect(nt("a?: string")).toEqual({ name: "a", type: "string", optional: true }); + }); + it("extracts complex types without breaking on , : => or strings", function () { + expect(nt("items: Array>").type).toBe("Array>"); + expect(nt("cb: (x: number, y: string) => void").type).toBe("(x: number, y: string) => void"); + expect(nt("opts: { a: number; b: string }").type).toBe("{ a: number; b: string }"); + expect(nt("t: [number, string]").type).toBe("[number, string]"); + expect(nt('mode: "on" | "off"').type).toBe('"on" | "off"'); + expect(nt("x: T extends U ? A : B").type).toBe("T extends U ? A : B"); + expect(nt("cb: () => void = () => {}").type).toBe("() => void"); // default stripped + }); + it("falls back to a null type on an unbalanced/garbled annotation", function () { + expect(nt("a: Array' + }); + it("type-first (C/Java): trailing identifier is the name, no signature type", function () { + expect(nt("int x", "last")).toEqual({ name: "x", type: null, optional: false }); + expect(nt("const char *ptr", "last")).toEqual({ name: "ptr", type: null, optional: false }); + }); + }); + + describe("_validType", function () { + const valid = DocCommentHints._validType; + it("accepts balanced complex types", function () { + ["number", "Array>", "(x: T) => void", "{ a: number }", + "[a, b]", '"on" | "off"'].forEach(t => expect(valid(t)).toBe(true)); + }); + it("rejects empty or unbalanced types", function () { + ["", " ", "Array void", "}{"].forEach(t => expect(valid(t)).toBe(false)); }); }); describe("_parseSignature", function () { const parse = DocCommentHints._parseSignature; + const names = sig => sig.params.map(p => p.name); + const types = sig => sig.params.map(p => p.type); - it("parses a plain JS function (name-first, returns)", function () { + it("parses a plain JS function (name-first, returns, untyped)", function () { const sig = parse("function add(a, b) {", "first"); - expect(sig.params).toEqual(["a", "b"]); + expect(names(sig)).toEqual(["a", "b"]); + expect(types(sig)).toEqual([null, null]); expect(sig.isClass).toBe(false); expect(sig.hasReturn).toBe(true); }); - it("parses a typed TS function and keeps return", function () { + it("extracts param + return types from a typed TS function", function () { const sig = parse("function f(name: string, count: number = 1): boolean {", "first"); - expect(sig.params).toEqual(["name", "count"]); + expect(names(sig)).toEqual(["name", "count"]); + expect(types(sig)).toEqual(["string", "number"]); + expect(sig.returnType).toBe("boolean"); expect(sig.hasReturn).toBe(true); }); + it("handles a complex TS arrow (generic, function-type param, generic return)", function () { + const sig = parse("const f = (items: T[], cb: (x: T) => void): Promise => {", "first"); + expect(names(sig)).toEqual(["items", "cb"]); + expect(types(sig)).toEqual(["T[]", "(x: T) => void"]); + expect(sig.returnType).toBe("Promise"); + }); it("treats an explicit void return as no @returns", function () { expect(parse("function log(msg: string): void {", "first").hasReturn).toBe(false); }); it("skips self in a Python method", function () { - const sig = parse("def greet(self, name):", "first"); - expect(sig.params).toEqual(["name"]); + expect(names(parse("def greet(self, name):", "first"))).toEqual(["name"]); }); - it("parses a Java method type-first", function () { + it("parses a Java method type-first (names only, no signature type)", function () { const sig = parse("public int sum(int a, String b) {", "last"); - expect(sig.params).toEqual(["a", "b"]); + expect(names(sig)).toEqual(["a", "b"]); + expect(types(sig)).toEqual([null, null]); expect(sig.hasReturn).toBe(true); }); it("parses an arrow function", function () { - expect(parse("const mul = (a, b) => {", "first").params).toEqual(["a", "b"]); + expect(names(parse("const mul = (a, b) => {", "first"))).toEqual(["a", "b"]); }); it("handles a rest parameter", function () { - expect(parse("function f(a, ...rest) {", "first").params).toEqual(["a", "rest"]); + expect(names(parse("function f(a, ...rest) {", "first"))).toEqual(["a", "rest"]); }); it("reports a class with no params/return", function () { const sig = parse("export class Bar extends Base {", "first"); expect(sig.isClass).toBe(true); - expect(sig.params).toEqual([]); + expect(names(sig)).toEqual([]); expect(sig.hasReturn).toBe(false); }); it("treats a constructor as returning nothing", function () { const sig = parse("constructor(x, y) {", "first"); - expect(sig.params).toEqual(["x", "y"]); + expect(names(sig)).toEqual(["x", "y"]); expect(sig.hasReturn).toBe(false); }); it("handles an empty parameter list", function () { - expect(parse("function noop() {", "first").params).toEqual([]); + expect(names(parse("function noop() {", "first"))).toEqual([]); }); it("flags isDeclaration only for real declarations (gates the partial /, /* triggers)", function () { expect(parse("function add(a, b) {", "first").isDeclaration).toBe(true); @@ -115,6 +154,8 @@ define(function (require, exports, module) { describe("_buildSnippet", function () { const build = DocCommentHints._buildSnippet; + // Build the params array (objects now) from simple names. + const P = (...names) => names.map(n => ({ name: n })); // Every generated line must be free of trailing whitespace (linters flag it - see the // no-trailing-spaces report that prompted this). @@ -124,8 +165,8 @@ define(function (require, exports, module) { }); } - it("builds a JSDoc skeleton with type tabstops and no trailing whitespace", function () { - const snip = build("jsdoc", { params: ["a", "b"], isClass: false, hasReturn: true }, ""); + it("builds a JSDoc skeleton with {*} for untyped params and no trailing whitespace", function () { + const snip = build("jsdoc", { params: P("a", "b"), isClass: false, hasReturn: true }, ""); expect(snip.indexOf("/**")).toBe(0); expect(snip).toContain("${1:"); // summary tabstop (default text is localized) expect(snip).toContain("@param {${2:*}} a"); @@ -134,6 +175,22 @@ define(function (require, exports, module) { expect(snip.trim().slice(-2)).toBe("*/"); expectNoTrailingWhitespace(snip); }); + it("fills the real type as the {type} tabstop, [name] for optional, typed @returns", function () { + const sig = { + params: [ + { name: "a", type: "number" }, + { name: "b", type: "Array" }, + { name: "c", type: "T", optional: true } + ], + returnType: "boolean", isClass: false, hasReturn: true + }; + const snip = build("jsdoc", sig, ""); + expect(snip).toContain("@param {${2:number}} a"); + expect(snip).toContain("@param {${3:Array}} b"); + expect(snip).toContain("@param {${4:T}} [c]"); + expect(snip).toContain("@returns {${5:boolean}}"); + expectNoTrailingWhitespace(snip); + }); it("omits @param/@returns for a class", function () { const snip = build("jsdoc", { params: [], isClass: true, hasReturn: false }, ""); expect(snip).toContain("${1:"); // summary tabstop only @@ -142,15 +199,15 @@ define(function (require, exports, module) { expectNoTrailingWhitespace(snip); }); it("indents continuation lines", function () { - const snip = build("jsdoc", { params: ["a"], isClass: false, hasReturn: false }, " "); + const snip = build("jsdoc", { params: P("a"), isClass: false, hasReturn: false }, " "); expect(snip).toContain("\n * @param"); // 4-space indent + " * " }); it("escapes $ in a PHP-style parameter name", function () { - const snip = build("jsdoc", { params: ["$user"], isClass: false, hasReturn: false }, ""); + const snip = build("jsdoc", { params: P("$user"), isClass: false, hasReturn: false }, ""); expect(snip).toContain("\\$user"); }); it("builds a PHPDoc skeleton (type before the $name, @return, no braces)", function () { - const snip = build("phpdoc", { params: ["$a", "$b"], isClass: false, hasReturn: true }, ""); + const snip = build("phpdoc", { params: P("$a", "$b"), isClass: false, hasReturn: true }, ""); expect(snip).toContain("@param ${2:mixed} \\$a"); expect(snip).toContain("@param ${3:mixed} \\$b"); expect(snip).toContain("@return ${4:mixed}"); @@ -158,7 +215,7 @@ define(function (require, exports, module) { expectNoTrailingWhitespace(snip); }); it("builds a Javadoc/Doxygen skeleton (no {type} braces, singular @return)", function () { - const snip = build("tagdoc", { params: ["a", "b"], isClass: false, hasReturn: true }, ""); + const snip = build("tagdoc", { params: P("a", "b"), isClass: false, hasReturn: true }, ""); expect(snip).toContain("@param a"); expect(snip).toContain("@param b"); expect(snip).toContain("@return"); @@ -167,7 +224,7 @@ define(function (require, exports, module) { expectNoTrailingWhitespace(snip); }); it("builds a Python docstring with Args/Returns and no trailing whitespace", function () { - const snip = build("pydoc", { params: ["name"], isClass: false, hasReturnType: true }, " "); + const snip = build("pydoc", { params: P("name"), isClass: false, hasReturnType: true }, " "); expect(snip.indexOf('"""')).toBe(0); expect(snip).toContain("Args:"); expect(snip).toContain("name: ${2:"); @@ -176,7 +233,7 @@ define(function (require, exports, module) { expectNoTrailingWhitespace(snip); }); it("Python omits Returns without an explicit return annotation", function () { - const snip = build("pydoc", { params: ["name"], isClass: false, hasReturnType: false }, " "); + const snip = build("pydoc", { params: P("name"), isClass: false, hasReturnType: false }, " "); expect(snip).toContain("Args:"); expect(snip).not.toContain("Returns:"); }); @@ -218,8 +275,9 @@ define(function (require, exports, module) { const CASES = [ { id: "javascript", content: "/**\nfunction f(a, b) {\n}\n", line: 0, ch: 3, label: "DOC_COMMENT_ADD_JSDOC", has: ["@param {*} a", "@param {*} b", "@returns {*}"], absent: [] }, - { id: "typescript", content: "/**\nfunction f(a) {\n}\n", line: 0, ch: 3, - label: "DOC_COMMENT_ADD_JSDOC", has: ["@param {*} a"], absent: [] }, + { id: "typescript", content: "/**\nfunction f(a: number, b: string): boolean {\n}\n", line: 0, ch: 3, + label: "DOC_COMMENT_ADD_JSDOC", + has: ["@param {number} a", "@param {string} b", "@returns {boolean}"], absent: ["{*}"] }, { id: "php", content: " Date: Tue, 30 Jun 2026 21:59:32 +0530 Subject: [PATCH 5/5] fix(codehints): TypeScript JSDoc carries names only, not {type} In a .ts file the parameter/return types live in the signature, so a JSDoc {type} (@param {number} / @returns {*}) is redundant and TypeScript flags it: "JSDoc types may be moved to TypeScript types." Our generator was raising that suggestion the instant a doc comment was inserted. Split the JSDoc style by language: - JavaScript/JSX -> jsdoc: @param {*} name / @returns {*} (JSDoc IS the JS type system; {*} is a valid placeholder in a .js file) - TypeScript/TSX -> new tsdoc style: @param name / @returns (names only, no {type} braces, no {*} on @returns - types come from the signature) The signature parser is unchanged and still extracts parameter names reliably from complex TS signatures (generics, function types, etc.); the type values are simply not emitted for TS. Tests: new tsdoc builder unit test; the TS provider case and the integration b.ts/c.ts cases now assert names-only and that {type}/{*} are absent; the integration suite now accepts the hint and asserts the inserted doc comment (not just that the popup appears). --- .../DocCommentHints/integration-tests.js | 67 +++++++++++++------ .../default/DocCommentHints/main.js | 48 +++++++------ .../default/DocCommentHints/unittests.js | 24 ++++++- 3 files changed, 98 insertions(+), 41 deletions(-) diff --git a/src/extensions/default/DocCommentHints/integration-tests.js b/src/extensions/default/DocCommentHints/integration-tests.js index 3c7688fb6c..f7c836473a 100644 --- a/src/extensions/default/DocCommentHints/integration-tests.js +++ b/src/extensions/default/DocCommentHints/integration-tests.js @@ -23,13 +23,27 @@ define(function (require, exports, module) { const SpecRunnerUtils = brackets.getModule("spec/SpecRunnerUtils"); - // Opener already present in each fixture; the test just parks the caret on it and asks for hints. - const LANGS = [ - { file: "a.js", line: 0, ch: 3, content: "/**\nfunction f(a, b) {\n}\n" }, - { file: "a.php", line: 1, ch: 3, content: " void): Promise {\n}\n", + expect: ["@param items", "@param cb", "@returns"], absent: ["@param {", "{T[]}", "{*}"] }, + { file: "a.php", line: 1, ch: 3, content: " 0; - }).length > 0; + }); } - LANGS.forEach(function (lang) { - it("shows the doc-comment hint popup for " + lang.file, async function () { - await awaitsForDone(SpecRunnerUtils.openProjectFiles([lang.file]), "open " + lang.file); + CASES.forEach(function (tc) { + it("shows the popup and inserts the right doc comment for " + tc.file, async function () { + await awaitsForDone(SpecRunnerUtils.openProjectFiles([tc.file]), "open " + tc.file); const editor = EditorManager.getActiveEditor(); - editor.setCursorPos(lang.line, lang.ch); + editor.setCursorPos(tc.line, tc.ch); CommandManager.execute(Commands.SHOW_CODE_HINTS); - await awaitsFor(docCommentHintVisible, - "doc-comment hint popup for " + lang.file, 5000); - expect(docCommentHintVisible()).toBe(true); - // Dismiss the popup before the next file so sessions don't bleed across specs. - const target = $(".codehint-menu:visible")[0] || testWindow.document.body; - SpecRunnerUtils.simulateKeyEvent(27, "keydown", target); + + // 1) the code-hints popup appears with our hint + await awaitsFor(function () { return $docHint().length > 0; }, + "doc-comment hint popup for " + tc.file, 5000); + + // 2) accepting it inserts that language's doc-comment convention + const $a = $docHint(); + $a.first().trigger("mousedown"); + $a.first().trigger("click"); + await awaitsFor(function () { + const text = editor.document.getText(); + return tc.expect.every(function (frag) { return text.indexOf(frag) !== -1; }); + }, "inserted doc comment for " + tc.file, 5000); + + const text = editor.document.getText(); + tc.expect.forEach(function (frag) { expect(text.indexOf(frag) !== -1).toBe(true); }); + (tc.absent || []).forEach(function (frag) { expect(text.indexOf(frag)).toBe(-1); }); }, 15000); }); }); diff --git a/src/extensions/default/DocCommentHints/main.js b/src/extensions/default/DocCommentHints/main.js index 2df6905fa2..59471d9646 100644 --- a/src/extensions/default/DocCommentHints/main.js +++ b/src/extensions/default/DocCommentHints/main.js @@ -48,11 +48,16 @@ define(function (require, exports, module) { const PROVIDER_PRIORITY = 100; // Doc-comment styles. The `/** */` syntax is shared, but the convention differs: - // jsdoc - @param {type} name / @returns {type} (JS/TS family) + // jsdoc - @param {type} name / @returns {type} (JavaScript: JSDoc IS the type system) + // tsdoc - @param name / @returns (TypeScript: types live in the + // signature, so a JSDoc {type} is + // redundant and TS flags it - + // "JSDoc types may be moved to TS types") // phpdoc - @param type $name / @return type (PHP: type before the $name) // tagdoc - @param name / @return (Javadoc & Doxygen: no {type} braces) // pydoc - """ ... Args:/Returns: ... """ (Python docstring) const STYLE_JSDOC = "jsdoc", + STYLE_TSDOC = "tsdoc", STYLE_PHPDOC = "phpdoc", STYLE_TAGDOC = "tagdoc", STYLE_PYDOC = "pydoc"; @@ -63,9 +68,9 @@ define(function (require, exports, module) { // intentionally absent: its doc comments are `///` markdown (# Arguments), not `/** @param */`. const LANGUAGES = { javascript: { style: STYLE_JSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, - typescript: { style: STYLE_JSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, + typescript: { style: STYLE_TSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, jsx: { style: STYLE_JSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, - tsx: { style: STYLE_JSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, + tsx: { style: STYLE_TSDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_JSDOC" }, php: { style: STYLE_PHPDOC, params: "first", opener: "block", label: "DOC_COMMENT_ADD_PHPDOC" }, java: { style: STYLE_TAGDOC, params: "last", opener: "block", label: "DOC_COMMENT_ADD_JAVADOC" }, c: { style: STYLE_TAGDOC, params: "last", opener: "block", label: "DOC_COMMENT_ADD_DOXYGEN" }, @@ -401,21 +406,25 @@ define(function (require, exports, module) { return out.join("\n"); } - // Javadoc / Doxygen: `@param name`, `@return` - no {type} braces (types come from the signature), - // and the singular `@return`. The only tabstop is the summary; param names are pre-filled. - function _buildTagDoc(sig, indent) { - const star = indent + " * "; - const out = ["/**", star + "${1:" + _escDesc(Strings.DOC_COMMENT_SUMMARY) + "}"]; - if (sig && !sig.isClass) { - sig.params.forEach(function (p) { - out.push(star + "@param " + _esc(p.name)); - }); - if (sig.hasReturn) { - out.push(star + "@return"); + // No-{type} block comment: `@param name` + a return tag, no {type} braces because the types live + // in the signature. Used by Javadoc/Doxygen (returnTag `@return`) and TypeScript (`@returns` - a + // JSDoc {type} in a .ts file is redundant and TS flags it). The only tabstop is the summary; + // param names are pre-filled. + function _buildTagDoc(returnTag) { + return function (sig, indent) { + const star = indent + " * "; + const out = ["/**", star + "${1:" + _escDesc(Strings.DOC_COMMENT_SUMMARY) + "}"]; + if (sig && !sig.isClass) { + sig.params.forEach(function (p) { + out.push(star + "@param " + _esc(p.name)); + }); + if (sig.hasReturn) { + out.push(star + returnTag); + } } - } - out.push(indent + " */"); - return out.join("\n"); + out.push(indent + " */"); + return out.join("\n"); + }; } function _buildPyDoc(sig, indent) { @@ -450,8 +459,9 @@ define(function (require, exports, module) { switch (style) { case STYLE_PYDOC: return _buildPyDoc(sig, indent); case STYLE_PHPDOC: return _buildPhpDoc(sig, indent); - case STYLE_TAGDOC: return _buildTagDoc(sig, indent); - default: return _buildJsDoc(sig, indent); + case STYLE_TAGDOC: return _buildTagDoc("@return")(sig, indent); // Javadoc / Doxygen + case STYLE_TSDOC: return _buildTagDoc("@returns")(sig, indent); // TypeScript (types in sig) + default: return _buildJsDoc(sig, indent); // JavaScript } } diff --git a/src/extensions/default/DocCommentHints/unittests.js b/src/extensions/default/DocCommentHints/unittests.js index 30b854e7e8..db4c037e92 100644 --- a/src/extensions/default/DocCommentHints/unittests.js +++ b/src/extensions/default/DocCommentHints/unittests.js @@ -223,6 +223,26 @@ define(function (require, exports, module) { expect(snip).not.toContain("@param {"); expectNoTrailingWhitespace(snip); }); + it("builds a TypeScript (tsdoc) skeleton - @param name / @returns, NO {type} braces", function () { + // TS types live in the signature, so a JSDoc {type} would be flagged by TS ("JSDoc types + // may be moved to TypeScript types"). tsdoc carries names only, with the JSDoc @returns. + const snip = build("tsdoc", { params: P("a", "b"), isClass: false, hasReturn: true }, ""); + expect(snip).toContain("@param a"); + expect(snip).toContain("@param b"); + expect(snip).toContain("@returns"); + expect(snip).not.toContain("@param {"); + expect(snip).not.toContain("{*}"); + expectNoTrailingWhitespace(snip); + }); + it("builds a TypeScript skeleton (no {type} braces, @returns) - types live in the signature", function () { + const snip = build("tsdoc", { params: P("a", "b"), isClass: false, hasReturn: true }, ""); + expect(snip).toContain("@param a"); + expect(snip).toContain("@param b"); + expect(snip).toContain("@returns"); + expect(snip).not.toContain("@param {"); // no JSDoc {type} -> no "JSDoc types may be moved" warning + expect(snip).not.toContain("{*}"); + expectNoTrailingWhitespace(snip); + }); it("builds a Python docstring with Args/Returns and no trailing whitespace", function () { const snip = build("pydoc", { params: P("name"), isClass: false, hasReturnType: true }, " "); expect(snip.indexOf('"""')).toBe(0); @@ -277,7 +297,9 @@ define(function (require, exports, module) { label: "DOC_COMMENT_ADD_JSDOC", has: ["@param {*} a", "@param {*} b", "@returns {*}"], absent: [] }, { id: "typescript", content: "/**\nfunction f(a: number, b: string): boolean {\n}\n", line: 0, ch: 3, label: "DOC_COMMENT_ADD_JSDOC", - has: ["@param {number} a", "@param {string} b", "@returns {boolean}"], absent: ["{*}"] }, + // TS types live in the signature, so JSDoc carries names only (no {type}, which TS + // would flag as "JSDoc types may be moved to TypeScript types"). + has: ["@param a", "@param b", "@returns"], absent: ["@param {", "{number}", "{*}"] }, { id: "php", content: "