From 373ad382b5ed7096d4f436231562ed159eada789 Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Fri, 20 May 2022 10:02:40 +1200 Subject: [PATCH] Analyzer CLI: Refactor main file (#33081) --- .../update-analyzer-command-refactor | 5 + plugins/woocommerce/composer.json | 2 +- .../src/commands/analyzer/index.ts | 273 ++---------------- tools/code-analyzer/src/git.ts | 85 ++++++ tools/code-analyzer/src/print.ts | 91 ++++++ tools/code-analyzer/src/utils.ts | 62 ++++ 6 files changed, 270 insertions(+), 248 deletions(-) create mode 100644 plugins/woocommerce/changelog/update-analyzer-command-refactor create mode 100644 tools/code-analyzer/src/git.ts create mode 100644 tools/code-analyzer/src/print.ts create mode 100644 tools/code-analyzer/src/utils.ts diff --git a/plugins/woocommerce/changelog/update-analyzer-command-refactor b/plugins/woocommerce/changelog/update-analyzer-command-refactor new file mode 100644 index 00000000000..39889b73336 --- /dev/null +++ b/plugins/woocommerce/changelog/update-analyzer-command-refactor @@ -0,0 +1,5 @@ +Significance: patch +Type: dev +Comment: Whitespace change, no changelog required + + diff --git a/plugins/woocommerce/composer.json b/plugins/woocommerce/composer.json index a4a965a33b0..3d94e2d7750 100644 --- a/plugins/woocommerce/composer.json +++ b/plugins/woocommerce/composer.json @@ -27,7 +27,7 @@ "require-dev": { "bamarni/composer-bin-plugin": "^1.4", "yoast/phpunit-polyfills": "^1.0", - "phpunit/phpunit": "7.5.20", + "phpunit/phpunit": "7.5.20", "automattic/jetpack-changelogger": "3.0.2" }, "config": { diff --git a/tools/code-analyzer/src/commands/analyzer/index.ts b/tools/code-analyzer/src/commands/analyzer/index.ts index 965f19b8dd1..de174898cfc 100644 --- a/tools/code-analyzer/src/commands/analyzer/index.ts +++ b/tools/code-analyzer/src/commands/analyzer/index.ts @@ -2,15 +2,21 @@ * External dependencies */ import { CliUx, Command, Flags } from '@oclif/core'; -import { tmpdir } from 'os'; import { join } from 'path'; import { readFileSync } from 'fs'; -import { execSync } from 'child_process'; /** * Internal dependencies */ import { MONOREPO_ROOT } from '../../const'; +import { printTemplateResults, printHookResults } from '../../print'; +import { + getVersionRegex, + getFilename, + getPatches, + getHookName, +} from '../../utils'; +import { generatePatch } from '../../git'; /** * Analyzer class @@ -69,10 +75,11 @@ export default class Analyzer extends Command { await this.validateArgs( flags.source ); - const patchContent = await this.getChanges( + const patchContent = await generatePatch( flags.source, args.compare, - flags.base + flags.base, + ( e: string ): void => this.error( e ) ); const pluginData = await this.getPluginData( flags.plugin ); @@ -152,129 +159,6 @@ export default class Analyzer extends Command { return [ version, pluginData.name, pluginData.mainFile ]; } - /** - * Fetch branches from origin. - * - * @param {string} branch branch/commit hash. - * @return {Promise} Promise. - */ - private async fetchBranch( branch: string ): Promise< boolean > { - CliUx.ux.action.start( `Fetching ${ branch }` ); - const branches = execSync( 'git branch', { - encoding: 'utf-8', - } ); - - const branchExistsLocally = branches.includes( branch ); - - if ( branchExistsLocally ) { - CliUx.ux.action.stop(); - return true; - } - - try { - // Fetch branch. - execSync( `git fetch origin ${ branch }` ); - // Create branch. - execSync( `git branch ${ branch } origin/${ branch }` ); - } catch ( e ) { - this.error( `Unable to fetch ${ branch }` ); - } - - CliUx.ux.action.stop(); - return true; - } - - /** - * Generate a patch file into the temp directory and return its contents - * - * @param {string} source The GitHub repository. - * @param {string} compare Branch/commit hash to compare against the base. - * @param {string} base Base branch/commit hash. - * @return {Promise} Promise. - */ - private async getChanges( - source: string, - compare: string, - base: string - ): Promise< string > { - const filename = `${ source }-${ base }-${ compare }.patch`.replace( - /\//g, - '-' - ); - const filepath = join( tmpdir(), filename ); - - await this.fetchBranch( base ); - await this.fetchBranch( compare ); - - CliUx.ux.action.start( 'Generating patch for ' + compare ); - - try { - const diffCommand = `git diff ${ base }...${ compare } > ${ filepath }`; - execSync( diffCommand ); - } catch ( e ) { - this.error( - 'Unable to create diff. Check that git origin, base branch, and compare branch all exist.' - ); - } - - const content = readFileSync( filepath ).toString(); - - CliUx.ux.action.stop(); - return content; - } - - /** - * Get patches - * - * @param {string} content Patch content. - * @param {RegExp} regex Regex to find specific patches. - * @return {Promise} Promise. - */ - private async getPatches( - content: string, - regex: RegExp - ): Promise< string[] > { - const patches = content.split( 'diff --git ' ); - const changes: string[] = []; - - for ( const p in patches ) { - const patch = patches[ p ]; - const id = patch.match( regex ); - - if ( id ) { - changes.push( patch ); - } - } - - return changes; - } - - /** - * Get filename from patch - * - * @param {string} str String to extract filename from. - * @return {Promise} Promise. - */ - private async getFilename( str: string ): Promise< string > { - return str.replace( /^a(.*)\s.*/, '$1' ); - } - - /** - * Format version string for regex. - * - * @param {string} rawVersion Raw version number. - * @return {Promise} Promise. - */ - private async getVersionRegex( rawVersion: string ): Promise< string > { - const version = rawVersion.replace( /\./g, '\\.' ); - - if ( rawVersion.endsWith( '.0' ) ) { - return version + '|' + version.slice( 0, -3 ) + '\\n'; - } - - return version; - } - /** * Scan patches for changes in templates, hooks and database schema * @@ -289,135 +173,30 @@ export default class Analyzer extends Command { ): Promise< void > { const templates = await this.scanTemplates( content, version ); const hooks = await this.scanHooks( content, version, output ); - // @todo: Scan for changes to database schema. if ( templates.size ) { - await this.printTemplateResults( + await printTemplateResults( templates, output, - 'TEMPLATE CHANGES' + 'TEMPLATE CHANGES', + ( s: string ): void => this.log( s ) ); } else { this.log( 'No template changes found' ); } if ( hooks.size ) { - await this.printHookResults( hooks, output, 'HOOKS' ); + await printHookResults( + hooks, + output, + 'HOOKS', + ( s: string ): void => this.log( s ) + ); } else { this.log( 'No new hooks found' ); } } - /** - * Print template results - * - * @param {Map} data Raw data. - * @param {string} output Output style. - * @param {string} title Section title. - */ - private async printTemplateResults( - data: Map< string, string[] >, - output: string, - title: string - ): Promise< void > { - if ( output === 'github' ) { - let opt = '\\n\\n### Template changes:'; - for ( const [ key, value ] of data ) { - opt += `\\n* **file:** ${ key }`; - opt += `\\n * ${ value[ 0 ].toUpperCase() }: ${ value[ 2 ] }`; - this.log( - `::${ value[ 0 ] } file=${ key },line=1,title=${ value[ 1 ] }::${ value[ 2 ] }` - ); - } - - this.log( `::set-output name=templates::${ opt }` ); - } else { - this.log( `\n## ${ title }:` ); - for ( const [ key, value ] of data ) { - this.log( 'FILE: ' + key ); - this.log( - '---------------------------------------------------' - ); - this.log( - ` ${ value[ 0 ].toUpperCase() } | ${ value[ 1 ] } | ${ - value[ 2 ] - }` - ); - this.log( - '---------------------------------------------------' - ); - } - } - } - - /** - * Print hook results - * - * @param {Map} data Raw data. - * @param {string} output Output style. - * @param {string} title Section title. - */ - private async printHookResults( - data: Map< string, Map< string, string[] > >, - output: string, - title: string - ): Promise< void > { - if ( output === 'github' ) { - let opt = '\\n\\n### New hooks:'; - for ( const [ key, value ] of data ) { - if ( value.size ) { - opt += `\\n* **file:** ${ key }`; - for ( const [ k, v ] of value ) { - opt += `\\n * ${ v[ 0 ].toUpperCase() }: ${ v[ 2 ] }`; - this.log( - `::${ v[ 0 ] } file=${ key },line=1,title=${ v[ 1 ] } - ${ k }::${ v[ 2 ] }` - ); - } - } - } - - this.log( `::set-output name=wphooks::${ opt }` ); - } else { - this.log( `\n## ${ title }:` ); - for ( const [ key, value ] of data ) { - if ( value.size ) { - this.log( 'FILE: ' + key ); - this.log( - '---------------------------------------------------' - ); - for ( const [ k, v ] of value ) { - this.log( 'HOOK: ' + k ); - this.log( - '---------------------------------------------------' - ); - this.log( - ` ${ v[ 0 ].toUpperCase() } | ${ v[ 1 ] } | ${ - v[ 2 ] - }` - ); - this.log( - '---------------------------------------------------' - ); - } - } - } - } - } - - /** - * Get hook name. - * - * @param {string} name Raw hook name. - * @return {Promise} Promise. - */ - private async getHookName( name: string ): Promise< string > { - if ( name.indexOf( ',' ) > -1 ) { - name = name.substring( 0, name.indexOf( ',' ) ); - } - - return name.replace( /(\'|\")/g, '' ).trim(); - } - /** * Scan patches for changes in templates * @@ -440,7 +219,7 @@ export default class Analyzer extends Command { const matchPatches = /^a\/(.+)\/templates\/(.+)/g; const title = 'Template change detected'; - const patches = await this.getPatches( content, matchPatches ); + const patches = getPatches( content, matchPatches ); const matchVersion = `^(\\+.+\\*.+)(@version)\\s+(${ version.replace( /\./g, '\\.' @@ -450,7 +229,7 @@ export default class Analyzer extends Command { for ( const p in patches ) { const patch = patches[ p ]; const lines = patch.split( '\n' ); - const filepath = await this.getFilename( lines[ 0 ] ); + const filepath = getFilename( lines[ 0 ] ); let code = 'warning'; let message = 'This template may require a version bump!'; @@ -500,8 +279,8 @@ export default class Analyzer extends Command { } const matchPatches = /^a\/(.+).php/g; - const patches = await this.getPatches( content, matchPatches ); - const verRegEx = await this.getVersionRegex( version ); + const patches = getPatches( content, matchPatches ); + const verRegEx = getVersionRegex( version ); const matchHooks = `@since\\s+(${ verRegEx })(.*?)(apply_filters|do_action)\\((\\s+)?(\\'|\\")(.*?)(\\'|\\")`; const newRegEx = new RegExp( matchHooks, 'gs' ); @@ -518,7 +297,7 @@ export default class Analyzer extends Command { } const lines = patch.split( '\n' ); - const filepath = await this.getFilename( lines[ 0 ] ); + const filepath = getFilename( lines[ 0 ] ); for ( const raw of results ) { // Extract hook name and type. @@ -530,7 +309,7 @@ export default class Analyzer extends Command { continue; } - const name = await this.getHookName( hookName[ 3 ] ); + const name = getHookName( hookName[ 3 ] ); const kind = hookName[ 2 ] === 'do_action' ? 'action' : 'filter'; const CLIMessage = `\'${ name }\' introduced in ${ version }`; diff --git a/tools/code-analyzer/src/git.ts b/tools/code-analyzer/src/git.ts new file mode 100644 index 00000000000..17f499729a9 --- /dev/null +++ b/tools/code-analyzer/src/git.ts @@ -0,0 +1,85 @@ +/** + * External dependencies + */ +import { CliUx } from '@oclif/core'; +import { execSync } from 'child_process'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { readFileSync } from 'fs'; + +/** + * Fetch branches from origin. + * + * @param {string} branch branch/commit hash. + * @param {Function} error error print method. + * @return {Promise} Promise. + */ +export const fetchBranch = async ( + branch: string, + error: ( s: string ) => void +): Promise< boolean > => { + CliUx.ux.action.start( `Fetching ${ branch }` ); + const branches = execSync( 'git branch', { + encoding: 'utf-8', + } ); + + const branchExistsLocally = branches.includes( branch ); + + if ( branchExistsLocally ) { + CliUx.ux.action.stop(); + return true; + } + + try { + // Fetch branch. + execSync( `git fetch origin ${ branch }` ); + // Create branch. + execSync( `git branch ${ branch } origin/${ branch }` ); + } catch ( e ) { + error( `Unable to fetch ${ branch }` ); + } + + CliUx.ux.action.stop(); + return true; +}; + +/** + * Generate a patch file into the temp directory and return its contents + * + * @param {string} source The GitHub repository. + * @param {string} compare Branch/commit hash to compare against the base. + * @param {string} base Base branch/commit hash. + * @param {Function} error error print method. + * @return {Promise} Promise. + */ +export const generatePatch = async ( + source: string, + compare: string, + base: string, + error: ( s: string ) => void +): Promise< string > => { + const filename = `${ source }-${ base }-${ compare }.patch`.replace( + /\//g, + '-' + ); + const filepath = join( tmpdir(), filename ); + + await fetchBranch( base, error ); + await fetchBranch( compare, error ); + + CliUx.ux.action.start( 'Generating patch for ' + compare ); + + try { + const diffCommand = `git diff ${ base }...${ compare } > ${ filepath }`; + execSync( diffCommand ); + } catch ( e ) { + error( + 'Unable to create diff. Check that git origin, base branch, and compare branch all exist.' + ); + } + + const content = readFileSync( filepath ).toString(); + + CliUx.ux.action.stop(); + return content; +}; diff --git a/tools/code-analyzer/src/print.ts b/tools/code-analyzer/src/print.ts new file mode 100644 index 00000000000..1b40bfe2d89 --- /dev/null +++ b/tools/code-analyzer/src/print.ts @@ -0,0 +1,91 @@ +/** + * Print template results + * + * @param {Map} data Raw data. + * @param {string} output Output style. + * @param {string} title Section title. + * @param {Function} log print method. + */ +export const printTemplateResults = async ( + data: Map< string, string[] >, + output: string, + title: string, + log: ( s: string ) => void +): Promise< void > => { + if ( output === 'github' ) { + let opt = '\\n\\n### Template changes:'; + for ( const [ key, value ] of data ) { + opt += `\\n* **file:** ${ key }`; + opt += `\\n * ${ value[ 0 ].toUpperCase() }: ${ value[ 2 ] }`; + log( + `::${ value[ 0 ] } file=${ key },line=1,title=${ value[ 1 ] }::${ value[ 2 ] }` + ); + } + + log( `::set-output name=templates::${ opt }` ); + } else { + log( `\n## ${ title }:` ); + for ( const [ key, value ] of data ) { + log( 'FILE: ' + key ); + log( '---------------------------------------------------' ); + log( + ` ${ value[ 0 ].toUpperCase() } | ${ value[ 1 ] } | ${ + value[ 2 ] + }` + ); + log( '---------------------------------------------------' ); + } + } +}; + +/** + * Print hook results + * + * @param {Map} data Raw data. + * @param {string} output Output style. + * @param {string} title Section title. + * @param {Function} log print method. + */ +export const printHookResults = async ( + data: Map< string, Map< string, string[] > >, + output: string, + title: string, + log: ( s: string ) => void +): Promise< void > => { + if ( output === 'github' ) { + let opt = '\\n\\n### New hooks:'; + for ( const [ key, value ] of data ) { + if ( value.size ) { + opt += `\\n* **file:** ${ key }`; + for ( const [ k, v ] of value ) { + opt += `\\n * ${ v[ 0 ].toUpperCase() }: ${ v[ 2 ] }`; + log( + `::${ v[ 0 ] } file=${ key },line=1,title=${ v[ 1 ] } - ${ k }::${ v[ 2 ] }` + ); + } + } + } + + log( `::set-output name=wphooks::${ opt }` ); + } else { + log( `\n## ${ title }:` ); + for ( const [ key, value ] of data ) { + if ( value.size ) { + log( 'FILE: ' + key ); + log( '---------------------------------------------------' ); + for ( const [ k, v ] of value ) { + log( 'HOOK: ' + k ); + log( + '---------------------------------------------------' + ); + log( + ` ${ v[ 0 ].toUpperCase() } | ${ v[ 1 ] } | ${ v[ 2 ] }` + ); + log( + '---------------------------------------------------' + ); + } + } + } + } +}; diff --git a/tools/code-analyzer/src/utils.ts b/tools/code-analyzer/src/utils.ts new file mode 100644 index 00000000000..e1c19755c71 --- /dev/null +++ b/tools/code-analyzer/src/utils.ts @@ -0,0 +1,62 @@ +/** + * Format version string for regex. + * + * @param {string} rawVersion Raw version number. + * @return {string} version regex. + */ +export const getVersionRegex = ( rawVersion: string ): string => { + const version = rawVersion.replace( /\./g, '\\.' ); + + if ( rawVersion.endsWith( '.0' ) ) { + return version + '|' + version.slice( 0, -3 ) + '\\n'; + } + + return version; +}; + +/** + * Get filename from patch + * + * @param {string} str String to extract filename from. + * @return {string} formatted filename. + */ +export const getFilename = ( str: string ): string => { + return str.replace( /^a(.*)\s.*/, '$1' ); +}; + +/** + * Get patches + * + * @param {string} content Patch content. + * @param {RegExp} regex Regex to find specific patches. + * @return {string[]} Array of patches. + */ +export const getPatches = ( content: string, regex: RegExp ): string[] => { + const patches = content.split( 'diff --git ' ); + const changes: string[] = []; + + for ( const p in patches ) { + const patch = patches[ p ]; + const id = patch.match( regex ); + + if ( id ) { + changes.push( patch ); + } + } + + return changes; +}; + +/** + * Get hook name. + * + * @param {string} name Raw hook name. + * @return {string} Formatted hook name. + */ +export const getHookName = ( name: string ): string => { + if ( name.indexOf( ',' ) > -1 ) { + name = name.substring( 0, name.indexOf( ',' ) ); + } + + return name.replace( /(\'|\")/g, '' ).trim(); +};