Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Improve Promise Handling, Validation, and Code Efficiency Across Scripts #1403

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 41 additions & 40 deletions scripts/check-single-extension.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,46 +18,47 @@ exports.verifyExtension = async function (extensionName, options) {
if (!isValidExtensionName(extensionName))
return { code: 'invalid-file-name' };

const [community, reviewed] = await Promise.all([
readFile(`${extensionsFolder}/community/${extensionName}.json`).catch(
() => null
),
readFile(`${extensionsFolder}/reviewed/${extensionName}.json`).catch(
() => null
),
]);

if (!community && !reviewed) return { code: 'not-found' };
if (community && reviewed) return { code: 'duplicated' };

//@ts-ignore We know this cannot be null thanks to the checks done just before
/** @type {string} */ const file = (
community ? community : reviewed
).toString();

/** @type {any} */
let extension;
try {
extension = JSON.parse(file);
} catch {
return { code: 'invalid-json' };
const [community, reviewed] = await Promise.all([
// Added try-catch block for better async error handling
readFile(`${extensionsFolder}/community/${extensionName}.json`).catch(
() => null
),
readFile(`${extensionsFolder}/reviewed/${extensionName}.json`).catch(
() => null
),
]);
if (!community && !reviewed) return { code: 'not-found' };
if (community && reviewed) return { code: 'duplicated' };
//@ts-ignore We know this cannot be null thanks to the checks done just before
/** @type {string} */ const file = (
community ? community : reviewed
).toString();
/** @type {any} */
let extension;
try {
extension = JSON.parse(file);
} catch {
return { code: 'invalid-json' };
}
const validationDetails = await validateExtension(
{
state: 'success',
extension,
tier: community ? 'community' : 'reviewed',
filename: `${extensionName}.json`,
},
preliminaryCheck
);
if (validationDetails.length > 0)
return {
code: 'rule-break',
errors: validationDetails.map(({ message }) => message),
};
return { code: 'success' };
} catch (error) {
// General error catch for any unhandled async errors in the process
console.error(`Error verifying extension: ${extensionName}`, error);
return { code: 'not-found' }; // Return a default error code, you could also return something more specific based on the context
}

const validationDetails = await validateExtension(
{
state: 'success',
extension,
tier: community ? 'community' : 'reviewed',
filename: `${extensionName}.json`,
},
preliminaryCheck
);

if (validationDetails.length > 0)
return {
code: 'rule-break',
errors: validationDetails.map(({ message }) => message),
};

return { code: 'success' };
};
47 changes: 19 additions & 28 deletions scripts/extract-extension.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,40 +15,31 @@ exports.extractExtension = async function (
zipPath,
extensionsFolder = `${__dirname}/../extensions`
) {
// Load in the archive with JSZip
const zip = await JSZip.loadAsync(await readFile(zipPath)).catch((e) => {
console.warn(`JSZip loading error caught: `, e);
return null;
});
if (zip === null) return { error: 'zip-error' };

// Find JSON files in the zip top level folder
const jsonFiles = zip.file(/.*\.json$/);

// Ensure there is exactly 1 file
if (jsonFiles.length === 0) return { error: 'no-json-found' };
if (jsonFiles.length > 1) return { error: 'too-many-files' };
const [file] = jsonFiles;

const { name: extensionName, dir, ext } = parsePath(file.name);
if (ext !== '.json') return { error: 'invalid-file-name' };

// Ensure no special characters are in the extension name to prevent relative path
// name shenanigans with dots that could put the extension in the reviewed folder.
if (dir) return { error: 'invalid-file-name' };
if (!isValidExtensionName(extensionName))
return { error: 'invalid-file-name' };

try {
// Load in the archive with JSZip
const zip = await JSZip.loadAsync(await readFile(zipPath));
if (!zip) return { error: 'zip-error' };
// Find JSON files in the zip top level folder
const jsonFiles = zip.file(/.*\.json$/);
// Ensure there is exactly 1 file
if (jsonFiles.length === 0) return { error: 'no-json-found' };
if (jsonFiles.length > 1) return { error: 'too-many-files' };
const [file] = jsonFiles;
const { name: extensionName, dir, ext } = parsePath(file.name);
if (ext !== '.json') return { error: 'invalid-file-name' };
// Ensure no special characters are in the extension name to prevent relative path
// name shenanigans with dots that could put the extension in the reviewed folder.
if (dir) return { error: 'invalid-file-name' };
if (!isValidExtensionName(extensionName))
return { error: 'invalid-file-name' };
// Write the extension to the community extensions folder
await pipeline(
file.nodeStream(),
createWriteStream(`${extensionsFolder}/community/${file.name}`)
);
return { extensionName };
} catch (e) {
console.warn(`JSZip extraction error caught: `, e);
return { error: 'zip-error' };
console.warn(`JSZip loading or extraction error caught: `, e);
return { error: 'zip-error', details: e.message };
}

return { extensionName };
};
3 changes: 3 additions & 0 deletions scripts/lib/ExtensionNameValidator.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
/** @param {string} extensionName */
exports.isValidExtensionName = (extensionName) => {
// Ensure extensionName is a string to prevent runtime errors
if (typeof extensionName !== 'string') return false;

if (extensionName.length === 0) return false;

// Ensure that the first character is an uppercase character
Expand Down
4 changes: 2 additions & 2 deletions scripts/lib/rules/DotsInSentences.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ async function validate({ extension, publicEventsFunctions, onError }) {
func.functionType === 'ExpressionAndCondition'
? 'INSTRUCTION'
: func.functionType === 'ActionWithOperator'
? 'ACTION_WITH_OPERATOR'
: 'EXPRESSION',
? 'ACTION_WITH_OPERATOR'
: 'EXPRESSION',
`the function '${func.name}'`,
onError
);
Expand Down
4 changes: 2 additions & 2 deletions scripts/lib/rules/FilledOutDescriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ async function validate({ extension, publicEventsFunctions, onError }) {
func.functionType === 'ExpressionAndCondition'
? NECESSARY_FIELDS.INSTRUCTION
: func.functionType === 'ActionWithOperator'
? NECESSARY_FIELDS.ACTION_WITH_OPERATOR
: NECESSARY_FIELDS.EXPRESSION,
? NECESSARY_FIELDS.ACTION_WITH_OPERATOR
: NECESSARY_FIELDS.EXPRESSION,
`the function '${func.name}'`,
onError
);
Expand Down
14 changes: 12 additions & 2 deletions scripts/lib/rules/PascalCase.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,19 @@ async function validate({
publicEventsFunctions,
onError,
}) {
// Check if extension name is a valid string
if (typeof name !== 'string') {
onError('Invalid extension name. Expected a string.');
return;
}

if (legacyCamelCaseExtensions.has(name)) return;
for (const { name } of publicEventsFunctions) checkPascalCase(name, onError);
for (const { name } of eventsBasedBehaviors) checkPascalCase(name, onError);

// Combine both loops into one to reduce duplication
const allFunctions = [...publicEventsFunctions, ...eventsBasedBehaviors];
for (const { name } of allFunctions) {
checkPascalCase(name, onError);
}
}

/** @type {import("./rule").RuleModule} */
Expand Down