From eb2731cd7602650018a9fd8d71207472763583df Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Sun, 19 Aug 2018 14:03:21 -0400 Subject: [PATCH 1/9] tools: Include links to source code in documentation Parse soruce code using acorn; extracting exports. When producing documentation, match exports to headers. When a match is found, add a [src] link. This first commit handles simple exported classes and functions, and does so without requiring any changes to the source code or markdown. Subsequent commits will attempt to match more headers, and some of these changes are likely to require changes to the source code and/or markdown. --- Makefile | 7 +- doc/api_assets/style.css | 5 + test/doctool/test-doctool-html.js | 2 +- tools/doc/apilinks.js | 163 ++++++++++++++++++++++++++++++ tools/doc/generate.js | 7 +- tools/doc/html.js | 7 +- tools/doc/package-lock.json | 5 + tools/doc/package.json | 1 + 8 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 tools/doc/apilinks.js diff --git a/Makefile b/Makefile index 2452244fc25967..8769b5b95c9f58 100644 --- a/Makefile +++ b/Makefile @@ -645,10 +645,15 @@ out/doc/api/assets/%: doc/api_assets/% out/doc/api/assets run-npm-ci = $(PWD)/$(NPM) ci gen-api = tools/doc/generate.js --node-version=$(FULLVERSION) \ + --apilinks=out/apilinks.json \ --analytics=$(DOCS_ANALYTICS) $< --output-directory=out/doc/api +gen-apilink = tools/doc/apilinks.js $(wildcard lib/*.js) > $@ + +out/apilinks.json: $(wildcard lib/*.js) tools/doc/apilinks.js + $(call available-node, $(gen-apilink)) out/doc/api/%.json out/doc/api/%.html: doc/api/%.md tools/doc/generate.js \ - tools/doc/html.js tools/doc/json.js + tools/doc/html.js tools/doc/json.js | out/apilinks.json $(call available-node, $(gen-api)) out/doc/api/all.html: $(apidocs_html) tools/doc/allhtml.js diff --git a/doc/api_assets/style.css b/doc/api_assets/style.css index f59f3770048097..7d65b7405b41d1 100644 --- a/doc/api_assets/style.css +++ b/doc/api_assets/style.css @@ -283,6 +283,11 @@ h2, h3, h4, h5 { padding-right: 40px; } +.srclink { + float: right; + font-size: smaller; +} + h1 span, h2 span, h3 span, h4 span { position: absolute; display: block; diff --git a/test/doctool/test-doctool-html.js b/test/doctool/test-doctool-html.js index 8c05ea6a0b3229..2fcb8315afd325 100644 --- a/test/doctool/test-doctool-html.js +++ b/test/doctool/test-doctool-html.js @@ -28,7 +28,7 @@ function toHTML({ input, filename, nodeVersion, analytics }, cb) { .use(html.firstHeader) .use(html.preprocessText) .use(html.preprocessElements, { filename }) - .use(html.buildToc, { filename }) + .use(html.buildToc, { filename, apilinks: {} }) .use(remark2rehype, { allowDangerousHTML: true }) .use(raw) .use(htmlStringify) diff --git a/tools/doc/apilinks.js b/tools/doc/apilinks.js new file mode 100644 index 00000000000000..6e328259c145e1 --- /dev/null +++ b/tools/doc/apilinks.js @@ -0,0 +1,163 @@ +'use strict'; + +// Scan API sources for definitions. +// +// Note the output is produced based on a world class parser, adherence to +// conventions, and a bit of guess work. Examples: +// +// * We scan for top level module.exports statements, and determine what +// is exported by looking at the source code only (i.e., we don't do +// an eval). If exports include `Foo`, it probably is a class, whereas +// if what is exported is `constants` it probably is prefixed by the +// by the basename of the source file (e.g., `zlib`), unless that +// source file is `buffer.js`, in which case the name is just `buf`. +// unless the constant is `kMaxLength`, in which case it is `buffer`. +// +// * We scan for top level definitions for those exports, handling +// most common cases (e.g., `X.prototype.foo =`, `X.foo =`, +// `function X(...) {...}`). Over time, we expect to handle more +// cases (example: ES2015 class definitions). + +const acorn = require('acorn'); +const fs = require('fs'); +const path = require('path'); +const child_process = require('child_process'); + +// determine orign repo and most recent commit +const repo = ( + child_process.execSync( + 'git config remote.origin.url', + { stdio: ['ignore', null, 'ignore'] } + ).toString() + .match(/(\w+\/\w+)\.git\r?\n?$/) || ['', 'nodejs/node'])[1]; + +let hash = child_process.execSync( + 'git log -1 --pretty=%H', + { stdio: ['ignore', null, 'ignore'] } +).toString().trim() || 'master'; + +try { + // replace hash with tag name, if tagged + hash = child_process.execSync( + `git describe --contains ${hash}`, + { stdio: ['ignore', null, 'ignore'] } + ).toString(); +} catch { +} + +const definition = {}; +process.argv.slice(2).forEach((file) => { + const basename = path.basename(file, '.js'); + + // parse source + const source = fs.readFileSync(file, 'utf8'); + const ast = acorn.parse(source, { ecmaVersion: 10 }); + const program = ast.body; + + // scan for line termination characters + const eol = /\n/g; + let match; + const lineends = [0]; + while (match = eol.exec(source)) { + lineends.push(match.index); + } + + // Binary search lineends to find the line number. Note that line numbers + // start with one (that's why lineends has an extra zero prepended to the + // list), and that the end of the loop, min and max point to the line ends + // immediately before and immediately after the indicated position. + function lineno(position) { + var min = 0; + var max = lineends.length; + while (max - 1 > min) { + const mid = Math.floor((min + max) / 2); + if (position > lineends[mid]) { + min = mid; + } else { + max = mid; + } + } + return max; + } + + // scan for exports + const exported = { constructors: [], identifiers: [] }; + program.forEach((statement) => { + if (statement.type !== 'ExpressionStatement') return; + const expr = statement.expression; + if (expr.type !== 'AssignmentExpression') return; + + let lhs = expr.left; + if (expr.left.object.type === 'MemberExpression') lhs = lhs.object; + if (lhs.type !== 'MemberExpression') return; + if (lhs.object.name !== 'module') return; + if (lhs.property.name !== 'exports') return; + + let rhs = expr.right; + while (rhs.type === 'AssignmentExpression') rhs = rhs.right; + + if (rhs.type === 'NewExpression') { + exported.constructors.push(rhs.callee.name); + } else if (rhs.type === 'ObjectExpression') { + rhs.properties.forEach((property) => { + if (property.value.type === 'Identifier') { + exported.identifiers.push(property.value.name); + if (/^[A-Z]/.test(property.value.name[0])) { + exported.constructors.push(property.value.name); + } + } + }); + } else if (rhs.type === 'Identifier') { + exported.identifiers.push(rhs.name); + } + }); + + // scan for definitions matching those exports; currently supports: + // + // ClassName.foo = ...; + // ClassName.prototype.foo = ...; + // function Identifier(...) {...}; + // + const link = `https://github.com/${repo}/blob/${hash}/${file}`; + program.forEach((statement) => { + if (statement.type === 'ExpressionStatement') { + const expr = statement.expression; + if (expr.type !== 'AssignmentExpression') return; + if (expr.left.type !== 'MemberExpression') return; + + let object; + if (expr.left.object.type === 'MemberExpression') { + if (expr.left.object.property.name !== 'prototype') return; + object = expr.left.object.object; + } else if (expr.left.object.type === 'Identifier') { + object = expr.left.object; + } else { + return; + } + + if (!exported.constructors.includes(object.name)) return; + + let objectName = object.name; + if (expr.left.object.type === 'MemberExpression') { + objectName = objectName.toLowerCase(); + if (objectName === 'buffer') objectName = 'buf'; + } + + let name = expr.left.property.name; + if (expr.left.computed) { + name = `${objectName}[${name}]`; + } else { + name = `${objectName}.${name}`; + } + + definition[name] = `${link}#L${lineno(statement.start)}`; + } else if (statement.type === 'FunctionDeclaration') { + const name = statement.id.name; + if (!exported.identifiers.includes(name)) return; + if (basename.startsWith('_')) return; + definition[`${basename}.${name}`] = `${link}#L${lineno(statement.start)}`; + } + }); +}); + +console.log(JSON.stringify(definition, null, 2)); diff --git a/tools/doc/generate.js b/tools/doc/generate.js index f2e3e8a1649985..28e09d003fa6db 100644 --- a/tools/doc/generate.js +++ b/tools/doc/generate.js @@ -40,6 +40,7 @@ let filename = null; let nodeVersion = null; let analytics = null; let outputDir = null; +let apilinks = {}; args.forEach(function(arg) { if (!arg.startsWith('--')) { @@ -50,6 +51,10 @@ args.forEach(function(arg) { analytics = arg.replace(/^--analytics=/, ''); } else if (arg.startsWith('--output-directory=')) { outputDir = arg.replace(/^--output-directory=/, ''); + } else if (arg.startsWith('--apilinks=')) { + apilinks = JSON.parse( + fs.readFileSync(arg.replace(/^--apilinks=/, ''), 'utf8') + ); } }); @@ -71,7 +76,7 @@ fs.readFile(filename, 'utf8', (er, input) => { .use(json.jsonAPI, { filename }) .use(html.firstHeader) .use(html.preprocessElements, { filename }) - .use(html.buildToc, { filename }) + .use(html.buildToc, { filename, apilinks }) .use(remark2rehype, { allowDangerousHTML: true }) .use(raw) .use(htmlStringify) diff --git a/tools/doc/html.js b/tools/doc/html.js index f1ac9e144e61e1..899605437ce7b3 100644 --- a/tools/doc/html.js +++ b/tools/doc/html.js @@ -329,7 +329,7 @@ function versionSort(a, b) { return +b.match(numberRe)[0] - +a.match(numberRe)[0]; } -function buildToc({ filename }) { +function buildToc({ filename, apilinks }) { return (tree, file) => { const startIncludeRefRE = /^\s*\s*$/; const endIncludeRefRE = /^\s*\s*$/; @@ -376,6 +376,11 @@ function buildToc({ filename }) { `id="${headingText}">#`; } + const api = headingText.replace(/^.*:\s+/, '').replace(/\(.*/, ''); + if (apilinks[api]) { + anchor = `[src]${anchor}`; + } + node.children.push({ type: 'html', value: anchor }); }); diff --git a/tools/doc/package-lock.json b/tools/doc/package-lock.json index c688e60c1577c8..c6fd0c7de73c91 100644 --- a/tools/doc/package-lock.json +++ b/tools/doc/package-lock.json @@ -9,6 +9,11 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-10.3.5.tgz", "integrity": "sha512-6lRwZN0Y3TuglwaaZN2XPocobmzLlhxcqDjKFjNYSsXG/TFAGYkCqkzZh4+ms8iTHHQE6gJXLHPV7TziVGeWhg==" }, + "acorn": { + "version": "5.7.1", + "resolved": "https://registry.npmjs.org/acorn/-/acorn-5.7.1.tgz", + "integrity": "sha512-d+nbxBUGKg7Arpsvbnlq61mc12ek3EY8EQldM3GPAhWJ1UVxC6TDGbIvUMNU6obBX3i1+ptCIzV4vq0gFPEGVQ==" + }, "argparse": { "version": "1.0.10", "resolved": "https://registry.npmjs.org/argparse/-/argparse-1.0.10.tgz", diff --git a/tools/doc/package.json b/tools/doc/package.json index 41ae87889d36e4..6a992e9dc7c38b 100644 --- a/tools/doc/package.json +++ b/tools/doc/package.json @@ -7,6 +7,7 @@ "node": ">=6" }, "dependencies": { + "acorn": "^5.7.1", "rehype-raw": "^2.0.0", "rehype-stringify": "^3.0.0", "remark-html": "^7.0.0", From 55b315821f5679a3e376a8a810daeb0c19879e13 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Sun, 26 Aug 2018 18:27:13 -0400 Subject: [PATCH 2/9] address a number of comments --- tools/doc/apilinks.js | 56 ++++++++++--------------------------- tools/doc/package-lock.json | 5 ---- tools/doc/package.json | 1 - 3 files changed, 15 insertions(+), 47 deletions(-) diff --git a/tools/doc/apilinks.js b/tools/doc/apilinks.js index 6e328259c145e1..91f1d078df625f 100644 --- a/tools/doc/apilinks.js +++ b/tools/doc/apilinks.js @@ -3,27 +3,27 @@ // Scan API sources for definitions. // // Note the output is produced based on a world class parser, adherence to -// conventions, and a bit of guess work. Examples: +// conventions, and a bit of guess work. Examples: // // * We scan for top level module.exports statements, and determine what // is exported by looking at the source code only (i.e., we don't do -// an eval). If exports include `Foo`, it probably is a class, whereas +// an eval). If exports include `Foo`, it probably is a class, whereas // if what is exported is `constants` it probably is prefixed by the -// by the basename of the source file (e.g., `zlib`), unless that -// source file is `buffer.js`, in which case the name is just `buf`. -// unless the constant is `kMaxLength`, in which case it is `buffer`. +// basename of the source file (e.g., `zlib`), unless that source file is +// `buffer.js`, in which case the name is just `buf`. unless the constant +// is `kMaxLength`, in which case it is `buffer`. // // * We scan for top level definitions for those exports, handling // most common cases (e.g., `X.prototype.foo =`, `X.foo =`, -// `function X(...) {...}`). Over time, we expect to handle more +// `function X(...) {...}`). Over time, we expect to handle more // cases (example: ES2015 class definitions). -const acorn = require('acorn'); +const acorn = require('../../deps/acorn'); const fs = require('fs'); const path = require('path'); const child_process = require('child_process'); -// determine orign repo and most recent commit +// Determine orign repo and most recent commit. const repo = ( child_process.execSync( 'git config remote.origin.url', @@ -37,7 +37,7 @@ let hash = child_process.execSync( ).toString().trim() || 'master'; try { - // replace hash with tag name, if tagged + // Replace hash with tag name, if tagged. hash = child_process.execSync( `git describe --contains ${hash}`, { stdio: ['ignore', null, 'ignore'] } @@ -49,38 +49,12 @@ const definition = {}; process.argv.slice(2).forEach((file) => { const basename = path.basename(file, '.js'); - // parse source + // Parse source. const source = fs.readFileSync(file, 'utf8'); - const ast = acorn.parse(source, { ecmaVersion: 10 }); + const ast = acorn.parse(source, { ecmaVersion: 10, locations: true }); const program = ast.body; - // scan for line termination characters - const eol = /\n/g; - let match; - const lineends = [0]; - while (match = eol.exec(source)) { - lineends.push(match.index); - } - - // Binary search lineends to find the line number. Note that line numbers - // start with one (that's why lineends has an extra zero prepended to the - // list), and that the end of the loop, min and max point to the line ends - // immediately before and immediately after the indicated position. - function lineno(position) { - var min = 0; - var max = lineends.length; - while (max - 1 > min) { - const mid = Math.floor((min + max) / 2); - if (position > lineends[mid]) { - min = mid; - } else { - max = mid; - } - } - return max; - } - - // scan for exports + // Scan for exports. const exported = { constructors: [], identifiers: [] }; program.forEach((statement) => { if (statement.type !== 'ExpressionStatement') return; @@ -112,7 +86,7 @@ process.argv.slice(2).forEach((file) => { } }); - // scan for definitions matching those exports; currently supports: + // Scan for definitions matching those exports; currently supports: // // ClassName.foo = ...; // ClassName.prototype.foo = ...; @@ -150,12 +124,12 @@ process.argv.slice(2).forEach((file) => { name = `${objectName}.${name}`; } - definition[name] = `${link}#L${lineno(statement.start)}`; + definition[name] = `${link}#L${statement.start.line}`; } else if (statement.type === 'FunctionDeclaration') { const name = statement.id.name; if (!exported.identifiers.includes(name)) return; if (basename.startsWith('_')) return; - definition[`${basename}.${name}`] = `${link}#L${lineno(statement.start)}`; + definition[`${basename}.${name}`] = `${link}#L${statement.start.line}`; } }); }); diff --git a/tools/doc/package-lock.json b/tools/doc/package-lock.json index c6fd0c7de73c91..c688e60c1577c8 100644 --- a/tools/doc/package-lock.json +++ b/tools/doc/package-lock.json @@ -9,11 +9,6 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-10.3.5.tgz", "integrity": "sha512-6lRwZN0Y3TuglwaaZN2XPocobmzLlhxcqDjKFjNYSsXG/TFAGYkCqkzZh4+ms8iTHHQE6gJXLHPV7TziVGeWhg==" }, - "acorn": { - "version": "5.7.1", - "resolved": "https://registry.npmjs.org/acorn/-/acorn-5.7.1.tgz", - "integrity": "sha512-d+nbxBUGKg7Arpsvbnlq61mc12ek3EY8EQldM3GPAhWJ1UVxC6TDGbIvUMNU6obBX3i1+ptCIzV4vq0gFPEGVQ==" - }, "argparse": { "version": "1.0.10", "resolved": "https://registry.npmjs.org/argparse/-/argparse-1.0.10.tgz", diff --git a/tools/doc/package.json b/tools/doc/package.json index 6a992e9dc7c38b..41ae87889d36e4 100644 --- a/tools/doc/package.json +++ b/tools/doc/package.json @@ -7,7 +7,6 @@ "node": ">=6" }, "dependencies": { - "acorn": "^5.7.1", "rehype-raw": "^2.0.0", "rehype-stringify": "^3.0.0", "remark-html": "^7.0.0", From 987a3c2845c6706daf956145fa771c823118e590 Mon Sep 17 00:00:00 2001 From: Refael Ackermann Date: Wed, 22 Aug 2018 16:15:52 -0400 Subject: [PATCH 3/9] tools,doc: test for apilinks --- test/doctool/test-apilinks.js | 24 ++++++++++++++++++++++++ test/fixtures/apilinks/known1.js | 21 +++++++++++++++++++++ tools/doc/apilinks.js | 5 +++-- 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 test/doctool/test-apilinks.js create mode 100644 test/fixtures/apilinks/known1.js diff --git a/test/doctool/test-apilinks.js b/test/doctool/test-apilinks.js new file mode 100644 index 00000000000000..7b4a897fb5bbd3 --- /dev/null +++ b/test/doctool/test-apilinks.js @@ -0,0 +1,24 @@ +'use strict'; + +require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +const script = path.join(__dirname, '..', '..', 'tools', 'doc', 'apilinks.js'); +const fixture = fixtures.path('apilinks', 'known1.js'); +const expected = { + 'Known.classField': 'known1.js#L7', + 'known1.createKnown': 'known1.js#L9' +}; +const ret = execFileSync( + process.execPath, [script, fixture], { encoding: 'utf-8' } +); +const links = JSON.parse(ret); +for (const [k, v] of Object.entries(expected)) { + assert.ok(k in links); + assert.ok(links[k].includes(v)); + delete links[k]; +} +assert.strictEqual(Object.keys(links).length, 0); diff --git a/test/fixtures/apilinks/known1.js b/test/fixtures/apilinks/known1.js new file mode 100644 index 00000000000000..1cd2bd8386cd05 --- /dev/null +++ b/test/fixtures/apilinks/known1.js @@ -0,0 +1,21 @@ +'use strict'; + +class Known { + getThing() {} +} + +Known.classField = 8 * 1024; + +function createKnown(size) { +} + +module.exports = { + Known, + createKnown +}; + +Object.defineProperty(exports, 'constants', { + configurable: false, + enumerable: true, + value: {k1: 1} +}); diff --git a/tools/doc/apilinks.js b/tools/doc/apilinks.js index 91f1d078df625f..303d65a972341b 100644 --- a/tools/doc/apilinks.js +++ b/tools/doc/apilinks.js @@ -124,12 +124,13 @@ process.argv.slice(2).forEach((file) => { name = `${objectName}.${name}`; } - definition[name] = `${link}#L${statement.start.line}`; + definition[name] = `${link}#L${statement.loc.start.line}`; } else if (statement.type === 'FunctionDeclaration') { const name = statement.id.name; if (!exported.identifiers.includes(name)) return; if (basename.startsWith('_')) return; - definition[`${basename}.${name}`] = `${link}#L${statement.start.line}`; + definition[`${basename}.${name}`] = + `${link}#L${statement.loc.start.line}`; } }); }); From df079ee649b7fc8021311e1e6e50f1078ca0066e Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Mon, 27 Aug 2018 14:52:17 -0400 Subject: [PATCH 4/9] [squashable] prepare for multiple apilinks tests 1) Run tests against every file in test/fixtures/apilinks/ 2) Embed expected results in the fixture itself --- test/doctool/test-apilinks.js | 43 +++++++++++++++++++++----------- test/fixtures/apilinks/known1.js | 5 ++++ 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/test/doctool/test-apilinks.js b/test/doctool/test-apilinks.js index 7b4a897fb5bbd3..562dcc5f9ab6eb 100644 --- a/test/doctool/test-apilinks.js +++ b/test/doctool/test-apilinks.js @@ -2,23 +2,36 @@ require('../common'); const fixtures = require('../common/fixtures'); +const fs = require('fs'); const assert = require('assert'); const path = require('path'); const { execFileSync } = require('child_process'); const script = path.join(__dirname, '..', '..', 'tools', 'doc', 'apilinks.js'); -const fixture = fixtures.path('apilinks', 'known1.js'); -const expected = { - 'Known.classField': 'known1.js#L7', - 'known1.createKnown': 'known1.js#L9' -}; -const ret = execFileSync( - process.execPath, [script, fixture], { encoding: 'utf-8' } -); -const links = JSON.parse(ret); -for (const [k, v] of Object.entries(expected)) { - assert.ok(k in links); - assert.ok(links[k].includes(v)); - delete links[k]; -} -assert.strictEqual(Object.keys(links).length, 0); + +const apilinks = fixtures.path('apilinks'); +fs.readdirSync(apilinks).forEach((fixture) => { + if (!fixture.endsWith('.js')) return; + const file = path.join(apilinks, fixture); + + let expected = {}; + eval(/(expected\s*=.*?);/s.exec(fs.readFileSync(file, 'utf8'))[1]); + + const links = JSON.parse(execFileSync( + process.execPath, + [script, file], + { encoding: 'utf-8' } + )); + + for (const [k, v] of Object.entries(expected)) { + assert.ok(k in links, `link not found: ${k}`); + assert.ok(links[k].endsWith('/' + v), + `link ${links[k]} expected to end with ${v}`); + delete links[k]; + } + + assert.strictEqual( + Object.keys(links).length, 0, + `unexpected links returned ${JSON.stringify(Object.keys(links))}` + ); +}); diff --git a/test/fixtures/apilinks/known1.js b/test/fixtures/apilinks/known1.js index 1cd2bd8386cd05..f859e1a3170c9b 100644 --- a/test/fixtures/apilinks/known1.js +++ b/test/fixtures/apilinks/known1.js @@ -19,3 +19,8 @@ Object.defineProperty(exports, 'constants', { enumerable: true, value: {k1: 1} }); + +const expected = { + "Known.classField": "known1.js#L7", + "known1.createKnown": "known1.js#L9" +}; From f59f014cd38853197df01912004fd99ecfbd16f0 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Mon, 27 Aug 2018 17:20:57 -0400 Subject: [PATCH 5/9] [squashable] attempt to address refack's comments I think I've addressed everything with the possible exception of the K.I.S.S. vs generic tester comment. See if this is acceptable. My thoughts are that we are going to end up with dozens of tests, and having a master list of expected results across all tests will be error prone. It is my expectation that it will be much easier to maintain individual tests and individual expected results. In the future, we can also parse process.argv to allow the running of an individual test. --- test/doctool/test-apilinks.js | 25 ++++++++++++++----------- test/fixtures/apilinks/known1.js | 5 ----- test/fixtures/apilinks/known1.json | 4 ++++ 3 files changed, 18 insertions(+), 16 deletions(-) create mode 100644 test/fixtures/apilinks/known1.json diff --git a/test/doctool/test-apilinks.js b/test/doctool/test-apilinks.js index 562dcc5f9ab6eb..a84ea74f59252f 100644 --- a/test/doctool/test-apilinks.js +++ b/test/doctool/test-apilinks.js @@ -14,24 +14,27 @@ fs.readdirSync(apilinks).forEach((fixture) => { if (!fixture.endsWith('.js')) return; const file = path.join(apilinks, fixture); - let expected = {}; - eval(/(expected\s*=.*?);/s.exec(fs.readFileSync(file, 'utf8'))[1]); + const fixtureContent = fs.readFileSync(file, 'utf8'); + const expectedContent = fs.readFileSync(file+'on', 'utf8'); - const links = JSON.parse(execFileSync( + const output = execFileSync( process.execPath, [script, file], { encoding: 'utf-8' } - )); + ); + + const expectedLinks = JSON.parse(expectedContent); + const actualLinks = JSON.parse(output); - for (const [k, v] of Object.entries(expected)) { - assert.ok(k in links, `link not found: ${k}`); - assert.ok(links[k].endsWith('/' + v), - `link ${links[k]} expected to end with ${v}`); - delete links[k]; + for (const [k, v] of Object.entries(expectedLinks)) { + assert.ok(k in actualLinks, `link not found: ${k}`); + assert.ok(actualLinks[k].endsWith('/' + v), + `link ${actualLinks[k]} expected to end with ${v}`); + delete expectedLinks[k]; } assert.strictEqual( - Object.keys(links).length, 0, - `unexpected links returned ${JSON.stringify(Object.keys(links))}` + Object.keys(expectedLinks).length, 0, + `unexpected links returned ${JSON.stringify(Object.keys(expectedLinks))}` ); }); diff --git a/test/fixtures/apilinks/known1.js b/test/fixtures/apilinks/known1.js index f859e1a3170c9b..1cd2bd8386cd05 100644 --- a/test/fixtures/apilinks/known1.js +++ b/test/fixtures/apilinks/known1.js @@ -19,8 +19,3 @@ Object.defineProperty(exports, 'constants', { enumerable: true, value: {k1: 1} }); - -const expected = { - "Known.classField": "known1.js#L7", - "known1.createKnown": "known1.js#L9" -}; diff --git a/test/fixtures/apilinks/known1.json b/test/fixtures/apilinks/known1.json new file mode 100644 index 00000000000000..0dfd9967d8b461 --- /dev/null +++ b/test/fixtures/apilinks/known1.json @@ -0,0 +1,4 @@ +{ + "Known.classField": "known1.js#L7", + "known1.createKnown": "known1.js#L9" +} From 9c697eb73650c7351bbf6e1550056a343815d5ca Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Mon, 27 Aug 2018 18:39:27 -0400 Subject: [PATCH 6/9] [squashable] appease linter --- test/doctool/test-apilinks.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/doctool/test-apilinks.js b/test/doctool/test-apilinks.js index a84ea74f59252f..3a4e2b8fdffb33 100644 --- a/test/doctool/test-apilinks.js +++ b/test/doctool/test-apilinks.js @@ -14,8 +14,7 @@ fs.readdirSync(apilinks).forEach((fixture) => { if (!fixture.endsWith('.js')) return; const file = path.join(apilinks, fixture); - const fixtureContent = fs.readFileSync(file, 'utf8'); - const expectedContent = fs.readFileSync(file+'on', 'utf8'); + const expectedContent = fs.readFileSync(file + 'on', 'utf8'); const output = execFileSync( process.execPath, From 99b11c4c09bdffabd319c20aa6af5958c8193f31 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Tue, 28 Aug 2018 16:17:35 -0400 Subject: [PATCH 7/9] relative paths and windows accomodation --- tools/doc/apilinks.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/doc/apilinks.js b/tools/doc/apilinks.js index 303d65a972341b..9c34f20d3d10d5 100644 --- a/tools/doc/apilinks.js +++ b/tools/doc/apilinks.js @@ -92,7 +92,9 @@ process.argv.slice(2).forEach((file) => { // ClassName.prototype.foo = ...; // function Identifier(...) {...}; // - const link = `https://github.com/${repo}/blob/${hash}/${file}`; + const link = `https://github.com/${repo}/blob/${hash}/` + + path.relative('.', file).replace(/\\/g, '/'); + program.forEach((statement) => { if (statement.type === 'ExpressionStatement') { const expr = statement.expression; From 8c21979eb79f4bfdd9dca47a5acbfabbf6a3c1b2 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Tue, 28 Aug 2018 17:58:27 -0400 Subject: [PATCH 8/9] [squashable] make github link determination more robust --- tools/doc/apilinks.js | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/tools/doc/apilinks.js b/tools/doc/apilinks.js index 9c34f20d3d10d5..98dd7827d67ac0 100644 --- a/tools/doc/apilinks.js +++ b/tools/doc/apilinks.js @@ -23,28 +23,29 @@ const fs = require('fs'); const path = require('path'); const child_process = require('child_process'); -// Determine orign repo and most recent commit. -const repo = ( - child_process.execSync( - 'git config remote.origin.url', - { stdio: ['ignore', null, 'ignore'] } - ).toString() - .match(/(\w+\/\w+)\.git\r?\n?$/) || ['', 'nodejs/node'])[1]; - -let hash = child_process.execSync( - 'git log -1 --pretty=%H', - { stdio: ['ignore', null, 'ignore'] } -).toString().trim() || 'master'; - -try { - // Replace hash with tag name, if tagged. - hash = child_process.execSync( - `git describe --contains ${hash}`, - { stdio: ['ignore', null, 'ignore'] } - ).toString(); -} catch { +// Run a command, capturing stdout, ignoring errors. +function execSync(command) { + try { + return child_process.execSync( + command, + { stdio: ['ignore', null, 'ignore'] } + ).toString().trim(); + } catch { + return ''; + } } +// Determine orign repo and tag (or hash) of the most recent commit. +const local_branch = execSync('git name-rev --name-only HEAD'); +const tracking_remote = execSync(`git config branch.${local_branch}.remote`); +const remote_url = execSync(`git config remote.${tracking_remote}.url`); +const repo = (remote_url.match(/(\w+\/\w+)\.git\r?\n?$/) || + ['', 'nodejs/node'])[1]; + +const hash = execSync('git log -1 --pretty=%H') || 'master'; +const tag = execSync(`git describe --contains ${hash}`).split('\n')[0] || hash; + +// Extract definitions from each file specified. const definition = {}; process.argv.slice(2).forEach((file) => { const basename = path.basename(file, '.js'); @@ -92,7 +93,7 @@ process.argv.slice(2).forEach((file) => { // ClassName.prototype.foo = ...; // function Identifier(...) {...}; // - const link = `https://github.com/${repo}/blob/${hash}/` + + const link = `https://github.com/${repo}/blob/${tag}/` + path.relative('.', file).replace(/\\/g, '/'); program.forEach((statement) => { From 242fb3b2eb8348ae4c1d7d6581dbc9d344b53e33 Mon Sep 17 00:00:00 2001 From: Sam Ruby Date: Wed, 29 Aug 2018 11:03:44 -0400 Subject: [PATCH 9/9] [squashable] split apilink tests into separate fixtures --- test/doctool/test-apilinks.js | 6 +++--- test/fixtures/apilinks/buffer.js | 12 ++++++++++++ test/fixtures/apilinks/buffer.json | 4 ++++ test/fixtures/apilinks/known1.js | 21 --------------------- test/fixtures/apilinks/known1.json | 4 ---- test/fixtures/apilinks/mod.js | 11 +++++++++++ test/fixtures/apilinks/mod.json | 3 +++ test/fixtures/apilinks/prototype.js | 13 +++++++++++++ test/fixtures/apilinks/prototype.json | 5 +++++ 9 files changed, 51 insertions(+), 28 deletions(-) create mode 100644 test/fixtures/apilinks/buffer.js create mode 100644 test/fixtures/apilinks/buffer.json delete mode 100644 test/fixtures/apilinks/known1.js delete mode 100644 test/fixtures/apilinks/known1.json create mode 100644 test/fixtures/apilinks/mod.js create mode 100644 test/fixtures/apilinks/mod.json create mode 100644 test/fixtures/apilinks/prototype.js create mode 100644 test/fixtures/apilinks/prototype.json diff --git a/test/doctool/test-apilinks.js b/test/doctool/test-apilinks.js index 3a4e2b8fdffb33..e53c81a08a7203 100644 --- a/test/doctool/test-apilinks.js +++ b/test/doctool/test-apilinks.js @@ -29,11 +29,11 @@ fs.readdirSync(apilinks).forEach((fixture) => { assert.ok(k in actualLinks, `link not found: ${k}`); assert.ok(actualLinks[k].endsWith('/' + v), `link ${actualLinks[k]} expected to end with ${v}`); - delete expectedLinks[k]; + delete actualLinks[k]; } assert.strictEqual( - Object.keys(expectedLinks).length, 0, - `unexpected links returned ${JSON.stringify(Object.keys(expectedLinks))}` + Object.keys(actualLinks).length, 0, + `unexpected links returned ${JSON.stringify(actualLinks)}` ); }); diff --git a/test/fixtures/apilinks/buffer.js b/test/fixtures/apilinks/buffer.js new file mode 100644 index 00000000000000..8ee44123a5e8ca --- /dev/null +++ b/test/fixtures/apilinks/buffer.js @@ -0,0 +1,12 @@ +'use strict'; + +// Buffer instance methods are exported as 'buf'. + +function Buffer() { +} + +Buffer.prototype.instanceMethod = function() {} + +module.exports = { + Buffer +}; diff --git a/test/fixtures/apilinks/buffer.json b/test/fixtures/apilinks/buffer.json new file mode 100644 index 00000000000000..528eca6eb21548 --- /dev/null +++ b/test/fixtures/apilinks/buffer.json @@ -0,0 +1,4 @@ +{ + "buffer.Buffer": "buffer.js#L5", + "buf.instanceMethod": "buffer.js#L8" +} diff --git a/test/fixtures/apilinks/known1.js b/test/fixtures/apilinks/known1.js deleted file mode 100644 index 1cd2bd8386cd05..00000000000000 --- a/test/fixtures/apilinks/known1.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict'; - -class Known { - getThing() {} -} - -Known.classField = 8 * 1024; - -function createKnown(size) { -} - -module.exports = { - Known, - createKnown -}; - -Object.defineProperty(exports, 'constants', { - configurable: false, - enumerable: true, - value: {k1: 1} -}); diff --git a/test/fixtures/apilinks/known1.json b/test/fixtures/apilinks/known1.json deleted file mode 100644 index 0dfd9967d8b461..00000000000000 --- a/test/fixtures/apilinks/known1.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "Known.classField": "known1.js#L7", - "known1.createKnown": "known1.js#L9" -} diff --git a/test/fixtures/apilinks/mod.js b/test/fixtures/apilinks/mod.js new file mode 100644 index 00000000000000..72606121f53fdb --- /dev/null +++ b/test/fixtures/apilinks/mod.js @@ -0,0 +1,11 @@ +'use strict'; + +// A module may export one or more methods. + +function foo() { +} + + +module.exports = { + foo +}; diff --git a/test/fixtures/apilinks/mod.json b/test/fixtures/apilinks/mod.json new file mode 100644 index 00000000000000..7d803e623672da --- /dev/null +++ b/test/fixtures/apilinks/mod.json @@ -0,0 +1,3 @@ +{ + "mod.foo": "mod.js#L5" +} diff --git a/test/fixtures/apilinks/prototype.js b/test/fixtures/apilinks/prototype.js new file mode 100644 index 00000000000000..40218e844e68ad --- /dev/null +++ b/test/fixtures/apilinks/prototype.js @@ -0,0 +1,13 @@ +'use strict'; + +// An exported class using classic prototype syntax. + +function Class() { +} + +Class.classMethod = function() {} +Class.prototype.instanceMethod = function() {} + +module.exports = { + Class +}; diff --git a/test/fixtures/apilinks/prototype.json b/test/fixtures/apilinks/prototype.json new file mode 100644 index 00000000000000..d61c32da62a64e --- /dev/null +++ b/test/fixtures/apilinks/prototype.json @@ -0,0 +1,5 @@ +{ + "prototype.Class": "prototype.js#L5", + "Class.classMethod": "prototype.js#L8", + "class.instanceMethod": "prototype.js#L9" +}