From fe707c335e27a101057ccb8be3dee4b8c825ea68 Mon Sep 17 00:00:00 2001 From: Gordon Williams Date: Tue, 28 Jun 2022 11:51:25 +0100 Subject: [PATCH] Add (hopefully!) github actions compatible error messages so we get files marked --- bin/sanitycheck.js | 161 ++++++++++++++++++++++++++------------------- 1 file changed, 93 insertions(+), 68 deletions(-) diff --git a/bin/sanitycheck.js b/bin/sanitycheck.js index 81c0f75ac..569f5fa4b 100755 --- a/bin/sanitycheck.js +++ b/bin/sanitycheck.js @@ -17,13 +17,21 @@ try { } var BASEDIR = __dirname+"/../"; -var APPSDIR = BASEDIR+"apps/"; -function ERROR(s) { - console.error("ERROR: "+s); - process.exit(1); +var APPSDIR_RELATIVE = "apps/"; +var APPSDIR = BASEDIR + APPSDIR_RELATIVE; +var warningCount = 0; +var errorCount = 0; +function ERROR(msg, opt) { + // file=app.js,line=1,col=5,endColumn=7 + opt = opt||{}; + console.log(`::error${Object.keys(opt).length?" ":""}${Object.keys(opt).map(k=>k+"="+opt[k]).join(",")}::${msg}`); + errorCount++; } -function WARN(s) { - console.log("Warning: "+s); +function WARN(msg, opt) { + // file=app.js,line=1,col=5,endColumn=7 + opt = opt||{}; + console.log(`::warning${Object.keys(opt).length?" ":""}${Object.keys(opt).map(k=>k+"="+opt[k]).join(",")}::${msg}`); + warningCount++; } var apps = []; @@ -43,16 +51,20 @@ dirs.forEach(dir => { } catch (e) { console.log(e); var m = e.toString().match(/in JSON at position (\d+)/); + var messageInfo = { + file : "apps/"+dir.name+"/metadata.json", + }; if (m) { var char = parseInt(m[1]); + messageInfo.line = appsFile.substr(0,char).split("\n").length; console.log("==============================================="); - console.log("LINE "+appsFile.substr(0,char).split("\n").length); + console.log("LINE "+messageInfo.line); console.log("==============================================="); console.log(appsFile.substr(char-10, 20)); console.log("==============================================="); } console.log(m); - ERROR(dir.name+"/metadata.json not valid JSON"); + ERROR(messageInfo.file+" not valid JSON", messageInfo); } }); @@ -87,87 +99,90 @@ let allFiles = []; let existingApps = []; apps.forEach((app,appIdx) => { if (!app.id) ERROR(`App ${appIdx} has no id`); - if (existingApps.includes(app.id)) ERROR(`Duplicate app '${app.id}'`); + var appDirRelative = APPSDIR_RELATIVE+app.id+"/"; + var appDir = APPSDIR+app.id+"/"; + var metadataFile = appDirRelative+"metadata.json"; + if (existingApps.includes(app.id)) ERROR(`Duplicate app '${app.id}'`, {file:metadataFile}); existingApps.push(app.id); //console.log(`Checking ${app.id}...`); - var appDir = APPSDIR+app.id+"/"; + if (!fs.existsSync(APPSDIR+app.id)) ERROR(`App ${app.id} has no directory`); - if (!app.name) ERROR(`App ${app.id} has no name`); + if (!app.name) ERROR(`App ${app.id} has no name`, {file:metadataFile}); var isApp = !app.type || app.type=="app"; - if (app.name.length>20 && !app.shortName && isApp) ERROR(`App ${app.id} has a long name, but no shortName`); + if (app.name.length>20 && !app.shortName && isApp) ERROR(`App ${app.id} has a long name, but no shortName`, {file:metadataFile}); if (app.type && !METADATA_TYPES.includes(app.type)) - ERROR(`App ${app.id} 'type' is one one of `+METADATA_TYPES); - if (!Array.isArray(app.supports)) ERROR(`App ${app.id} has no 'supports' field or it's not an array`); + ERROR(`App ${app.id} 'type' is one one of `+METADATA_TYPES, {file:metadataFile}); + if (!Array.isArray(app.supports)) ERROR(`App ${app.id} has no 'supports' field or it's not an array`, {file:metadataFile}); else { app.supports.forEach(dev => { if (!SUPPORTS_DEVICES.includes(dev)) - ERROR(`App ${app.id} has unknown device in 'supports' field - ${dev}`); + ERROR(`App ${app.id} has unknown device in 'supports' field - ${dev}`, {file:metadataFile}); }); } - if (!app.version) WARN(`App ${app.id} has no version`); + if (!app.version) ERROR(`App ${app.id} has no version`, {file:metadataFile}); else { if (!fs.existsSync(appDir+"ChangeLog")) { if (app.version != "0.01") - WARN(`App ${app.id} has no ChangeLog`); + WARN(`App ${app.id} has no ChangeLog`, {file:metadataFile}); } else { var changeLog = fs.readFileSync(appDir+"ChangeLog").toString(); var versions = changeLog.match(/\d+\.\d+:/g); - if (!versions) ERROR(`No versions found in ${app.id} ChangeLog (${appDir}ChangeLog)`); + if (!versions) ERROR(`No versions found in ${app.id} ChangeLog (${appDir}ChangeLog)`, {file:metadataFile}); var lastChangeLog = versions.pop().slice(0,-1); if (lastChangeLog != app.version) - WARN(`App ${app.id} app version (${app.version}) and ChangeLog (${lastChangeLog}) don't agree`); + ERROR(`App ${app.id} app version (${app.version}) and ChangeLog (${lastChangeLog}) don't agree`, {file:appDirRelative+"ChangeLog", line:changeLog.split("\n").length-1}); } } - if (!app.description) ERROR(`App ${app.id} has no description`); - if (!app.icon) ERROR(`App ${app.id} has no icon`); - if (!fs.existsSync(appDir+app.icon)) ERROR(`App ${app.id} icon doesn't exist`); + if (!app.description) ERROR(`App ${app.id} has no description`, {file:metadataFile}); + if (!app.icon) ERROR(`App ${app.id} has no icon`, {file:metadataFile}); + if (!fs.existsSync(appDir+app.icon)) ERROR(`App ${app.id} icon doesn't exist`, {file:metadataFile}); if (app.screenshots) { - if (!Array.isArray(app.screenshots)) ERROR(`App ${app.id} screenshots is not an array`); + if (!Array.isArray(app.screenshots)) ERROR(`App ${app.id} screenshots is not an array`, {file:metadataFile}); app.screenshots.forEach(screenshot => { if (!fs.existsSync(appDir+screenshot.url)) - ERROR(`App ${app.id} screenshot file ${screenshot.url} not found`); + ERROR(`App ${app.id} screenshot file ${screenshot.url} not found`, {file:metadataFile}); }); } - if (app.readme && !fs.existsSync(appDir+app.readme)) ERROR(`App ${app.id} README file doesn't exist`); - if (app.custom && !fs.existsSync(appDir+app.custom)) ERROR(`App ${app.id} custom HTML doesn't exist`); - if (app.customConnect && !app.custom) ERROR(`App ${app.id} has customConnect but no customn HTML`); - if (app.interface && !fs.existsSync(appDir+app.interface)) ERROR(`App ${app.id} interface HTML doesn't exist`); + if (app.readme && !fs.existsSync(appDir+app.readme)) ERROR(`App ${app.id} README file doesn't exist`, {file:metadataFile}); + if (app.custom && !fs.existsSync(appDir+app.custom)) ERROR(`App ${app.id} custom HTML doesn't exist`, {file:metadataFile}); + if (app.customConnect && !app.custom) ERROR(`App ${app.id} has customConnect but no customn HTML`, {file:metadataFile}); + if (app.interface && !fs.existsSync(appDir+app.interface)) ERROR(`App ${app.id} interface HTML doesn't exist`, {file:metadataFile}); if (app.dependencies) { if (("object"==typeof app.dependencies) && !Array.isArray(app.dependencies)) { Object.keys(app.dependencies).forEach(dependency => { if (!["type","app"].includes(app.dependencies[dependency])) - ERROR(`App ${app.id} 'dependencies' must all be tagged 'type' or 'app' right now`); + ERROR(`App ${app.id} 'dependencies' must all be tagged 'type' or 'app' right now`, {file:metadataFile}); if (app.dependencies[dependency]=="type" && !METADATA_TYPES.includes(dependency)) - ERROR(`App ${app.id} 'type' dependency must be one of `+METADATA_TYPES); + ERROR(`App ${app.id} 'type' dependency must be one of `+METADATA_TYPES, {file:metadataFile}); }); } else - ERROR(`App ${app.id} 'dependencies' must be an object`); + ERROR(`App ${app.id} 'dependencies' must be an object`, {file:metadataFile}); } var fileNames = []; app.storage.forEach((file) => { - if (!file.name) ERROR(`App ${app.id} has a file with no name`); - if (isGlob(file.name)) ERROR(`App ${app.id} storage file ${file.name} contains wildcards`); + if (!file.name) ERROR(`App ${app.id} has a file with no name`, {file:metadataFile}); + if (isGlob(file.name)) ERROR(`App ${app.id} storage file ${file.name} contains wildcards`, {file:metadataFile}); let char = file.name.match(FORBIDDEN_FILE_NAME_CHARS) - if (char) ERROR(`App ${app.id} storage file ${file.name} contains invalid character "${char[0]}"`) + if (char) ERROR(`App ${app.id} storage file ${file.name} contains invalid character "${char[0]}"`, {file:metadataFile}) if (fileNames.includes(file.name) && !file.supports) // assume that there aren't duplicates if 'supports' is set - ERROR(`App ${app.id} file ${file.name} is a duplicate`); + ERROR(`App ${app.id} file ${file.name} is a duplicate`, {file:metadataFile}); if (file.supports && !Array.isArray(file.supports)) - ERROR(`App ${app.id} file ${file.name} supports field must be an array`); + ERROR(`App ${app.id} file ${file.name} supports field must be an array`, {file:metadataFile}); if (file.supports) file.supports.forEach(dev => { if (!SUPPORTS_DEVICES.includes(dev)) - ERROR(`App ${app.id} file ${file.name} has unknown device in 'supports' field - ${dev}`); + ERROR(`App ${app.id} file ${file.name} has unknown device in 'supports' field - ${dev}`, {file:metadataFile}); }); fileNames.push(file.name); allFiles.push({app: app.id, file: file.name}); - if (file.url) if (!fs.existsSync(appDir+file.url)) ERROR(`App ${app.id} file ${file.url} doesn't exist`); - if (!file.url && !file.content && !app.custom) ERROR(`App ${app.id} file ${file.name} has no contents`); + if (file.url) if (!fs.existsSync(appDir+file.url)) ERROR(`App ${app.id} file ${file.url} doesn't exist`, {file:metadataFile}); + if (!file.url && !file.content && !app.custom) ERROR(`App ${app.id} file ${file.name} has no contents`, {file:metadataFile}); var fileContents = ""; if (file.content) fileContents = file.content; if (file.url) fileContents = fs.readFileSync(appDir+file.url).toString(); - if (file.supports && !Array.isArray(file.supports)) ERROR(`App ${app.id} file ${file.name} supports field is not an array`); + if (file.supports && !Array.isArray(file.supports)) ERROR(`App ${app.id} file ${file.name} supports field is not an array`, {file:metadataFile}); if (file.evaluate) { try { acorn.parse("("+fileContents+")"); @@ -179,7 +194,7 @@ apps.forEach((app,appIdx) => { console.log("====================================================="); console.log(fileContents); console.log("====================================================="); - ERROR(`App ${app.id}'s ${file.name} has evaluate:true but is not valid JS expression`); + ERROR(`App ${app.id}'s ${file.name} has evaluate:true but is not valid JS expression`, {file:appDirRelative+file.url}); } } if (file.name.endsWith(".js")) { @@ -194,11 +209,11 @@ apps.forEach((app,appIdx) => { console.log("====================================================="); console.log(fileContents); console.log("====================================================="); - ERROR(`App ${app.id}'s ${file.name} is a JS file but isn't valid JS`); + ERROR(`App ${app.id}'s ${file.name} is a JS file but isn't valid JS`, {file:appDirRelative+file.url}); } } for (const key in file) { - if (!STORAGE_KEYS.includes(key)) ERROR(`App ${app.id} file ${file.name} has unknown key ${key}`); + if (!STORAGE_KEYS.includes(key)) ERROR(`App ${app.id} file ${file.name} has unknown key ${key}`, {file:appDirRelative+file.url}); } // warn if JS icon is the wrong size if (file.name == app.id+".img") { @@ -209,44 +224,44 @@ apps.forEach((app,appIdx) => { else { match = fileContents.match(/^\s*require\(\"heatshrink\"\)\.decompress\(\s*atob\(\s*\"([^"]*)\"\s*\)\s*\)\s*$/); if (match) icon = heatshrink.decompress(Buffer.from(match[1], 'base64')); - else ERROR(`JS icon ${file.name} does not match the pattern 'require("heatshrink").decompress(atob("..."))'`); + else ERROR(`JS icon ${file.name} does not match the pattern 'require("heatshrink").decompress(atob("..."))'`, {file:appDirRelative+file.url}); } if (match) { if (icon[0] > 48 || icon[0] < 24 || icon[1] > 48 || icon[1] < 24) { - if (GRANDFATHERED_ICONS.includes(app.id)) WARN(`JS icon ${file.name} should be 48x48px (or slightly under) but is instead ${icon[0]}x${icon[1]}px`); - else ERROR(`JS icon ${file.name} should be 48x48px (or slightly under) but is instead ${icon[0]}x${icon[1]}px`); + if (GRANDFATHERED_ICONS.includes(app.id)) WARN(`JS icon ${file.name} should be 48x48px (or slightly under) but is instead ${icon[0]}x${icon[1]}px`, {file:appDirRelative+file.url}); + else ERROR(`JS icon ${file.name} should be 48x48px (or slightly under) but is instead ${icon[0]}x${icon[1]}px`, {file:appDirRelative+file.url}); } } } }); let dataNames = []; (app.data||[]).forEach((data)=>{ - if (!data.name && !data.wildcard) ERROR(`App ${app.id} has a data file with no name`); + if (!data.name && !data.wildcard) ERROR(`App ${app.id} has a data file with no name`, {file:metadataFile}); if (dataNames.includes(data.name||data.wildcard)) - ERROR(`App ${app.id} data file ${data.name||data.wildcard} is a duplicate`); + ERROR(`App ${app.id} data file ${data.name||data.wildcard} is a duplicate`, {file:metadataFile}); dataNames.push(data.name||data.wildcard) allFiles.push({app: app.id, data: (data.name||data.wildcard)}); if ('name' in data && 'wildcard' in data) - ERROR(`App ${app.id} data file ${data.name} has both name and wildcard`); + ERROR(`App ${app.id} data file ${data.name} has both name and wildcard`, {file:metadataFile}); if (isGlob(data.name)) - ERROR(`App ${app.id} data file name ${data.name} contains wildcards`); + ERROR(`App ${app.id} data file name ${data.name} contains wildcards`, {file:metadataFile}); if (data.wildcard) { if (!isGlob(data.wildcard)) - ERROR(`App ${app.id} data file wildcard ${data.wildcard} does not actually contains wildcard`); + ERROR(`App ${app.id} data file wildcard ${data.wildcard} does not actually contains wildcard`, {file:metadataFile}); if (data.wildcard.replace(/\?|\*/g,'') === '') - ERROR(`App ${app.id} data file wildcard ${data.wildcard} does not contain regular characters`); + ERROR(`App ${app.id} data file wildcard ${data.wildcard} does not contain regular characters`, {file:metadataFile}); else if (data.wildcard.replace(/\?|\*/g,'').length < 3) - WARN(`App ${app.id} data file wildcard ${data.wildcard} is very broad`); + WARN(`App ${app.id} data file wildcard ${data.wildcard} is very broad`, {file:metadataFile}); else if (!data.wildcard.includes(app.id)) - WARN(`App ${app.id} data file wildcard ${data.wildcard} does not include app ID`); + WARN(`App ${app.id} data file wildcard ${data.wildcard} does not include app ID`, {file:metadataFile}); } let char = (data.name||data.wildcard).match(FORBIDDEN_FILE_NAME_CHARS) - if (char) ERROR(`App ${app.id} data file ${data.name||data.wildcard} contains invalid character "${char[0]}"`) + if (char) ERROR(`App ${app.id} data file ${data.name||data.wildcard} contains invalid character "${char[0]}"`, {file:metadataFile}) if ('storageFile' in data && typeof data.storageFile !== 'boolean') - ERROR(`App ${app.id} data file ${data.name||data.wildcard} has non-boolean value for "storageFile"`); + ERROR(`App ${app.id} data file ${data.name||data.wildcard} has non-boolean value for "storageFile"`, {file:metadataFile}); for (const key in data) { if (!DATA_KEYS.includes(key)) - ERROR(`App ${app.id} data file ${data.name||data.wildcard} has unknown property "${key}"`); + ERROR(`App ${app.id} data file ${data.name||data.wildcard} has unknown property "${key}"`, {file:metadataFile}); } }); // prefer "appid.json" over "appid.settings.json" (TODO: change to ERROR once all apps comply?) @@ -256,32 +271,35 @@ apps.forEach((app,appIdx) => { WARN(`App ${app.id} uses data file ${app.id+'.settings.json'}`)*/ // settings files should be listed under data, not storage (TODO: change to ERROR once all apps comply?) if (fileNames.includes(app.id+".settings.json")) - WARN(`App ${app.id} uses storage file ${app.id+'.settings.json'} instead of data file`) + WARN(`App ${app.id} uses storage file ${app.id+'.settings.json'} instead of data file`, {file:metadataFile}) if (fileNames.includes(app.id+".json")) - WARN(`App ${app.id} uses storage file ${app.id+'.json'} instead of data file`) + WARN(`App ${app.id} uses storage file ${app.id+'.json'} instead of data file`, {file:metadataFile}) // warn if storage file matches data file of same app dataNames.forEach(dataName=>{ const glob = globToRegex(dataName) fileNames.forEach(fileName=>{ if (glob.test(fileName)) { - if (isGlob(dataName)) WARN(`App ${app.id} storage file ${fileName} matches data wildcard ${dataName}`) - else WARN(`App ${app.id} storage file ${fileName} is also listed in data`) + if (isGlob(dataName)) WARN(`App ${app.id} storage file ${fileName} matches data wildcard ${dataName}`, {file:metadataFile}) + else WARN(`App ${app.id} storage file ${fileName} is also listed in data`, {file:metadataFile}) } }) }) //console.log(fileNames); - if (isApp && !fileNames.includes(app.id+".app.js")) ERROR(`App ${app.id} has no entrypoint`); - if (isApp && !fileNames.includes(app.id+".img")) ERROR(`App ${app.id} has no JS icon`); - if (app.type=="widget" && !fileNames.includes(app.id+".wid.js")) ERROR(`Widget ${app.id} has no entrypoint`); + if (isApp && !fileNames.includes(app.id+".app.js")) ERROR(`App ${app.id} has no entrypoint`, {file:metadataFile}); + if (isApp && !fileNames.includes(app.id+".img")) ERROR(`App ${app.id} has no JS icon`, {file:metadataFile}); + if (app.type=="widget" && !fileNames.includes(app.id+".wid.js")) ERROR(`Widget ${app.id} has no entrypoint`, {file:metadataFile}); for (const key in app) { - if (!APP_KEYS.includes(key)) ERROR(`App ${app.id} has unknown key ${key}`); + if (!APP_KEYS.includes(key)) ERROR(`App ${app.id} has unknown key ${key}`, {file:metadataFile}); } }); + + // Do not allow files from different apps to collide let fileA + while(fileA=allFiles.pop()) { if (VALID_DUPLICATES.includes(fileA.file)) - return; + break; const nameA = (fileA.file||fileA.data), globA = globToRegex(nameA), typeA = fileA.file?'storage':'data' @@ -291,9 +309,16 @@ while(fileA=allFiles.pop()) { typeB = fileB.file?'storage':'data' if (globA.test(nameB)||globB.test(nameA)) { if (isGlob(nameA)||isGlob(nameB)) - ERROR(`App ${fileB.app} ${typeB} file ${nameB} matches app ${fileA.app} ${typeB} file ${nameA}`) + ERROR(`App ${fileB.app} ${typeB} file ${nameB} matches app ${fileA.app} ${typeB} file ${nameA}`); else if (fileA.app != fileB.app) - WARN(`App ${fileB.app} ${typeB} file ${nameB} is also listed as ${typeA} file for app ${fileA.app}`) + WARN(`App ${fileB.app} ${typeB} file ${nameB} is also listed as ${typeA} file for app ${fileA.app}`); } }) } + +console.log("=================================="); +console.log(`${errorCount} errors, ${warningCount} warnings`); +console.log("=================================="); +if (errorCount) { + process.exit(1); +}