From b716498eb2948ae4e87b0934c7046b850b62d10e Mon Sep 17 00:00:00 2001 From: Slava Leleka Date: Thu, 20 Jul 2023 12:43:44 +0300 Subject: [PATCH] update ModifierValidator for AGLint needs. AG-23003 Squashed commit of the following: commit 7beb82aaa41c2bc4f2bab807f1fa9811bc9bf829 Author: David Tota Date: Thu Jul 20 12:36:51 2023 +0300 fix error message commit 84a01f507a643b55ca74f2b85fec7b5e37deeea5 Merge: 7dd79b16 8b9101eb Author: Slava Leleka Date: Wed Jul 19 18:22:35 2023 +0300 Merge branch 'master' into fix/AG-23003-04 commit 7dd79b1620c4d441e473e65b8b92912f3c7a3f49 Merge: 8730bdb4 cb73a90d Author: Slava Leleka Date: Wed Jul 19 14:56:18 2023 +0300 Merge branch 'master' into fix/AG-23003-04 commit 8730bdb4cc323549bcbc4b0bfdda6fb11d4b079b Author: Slava Leleka Date: Wed Jul 19 14:30:37 2023 +0300 update ModifierValidator for AGLint needs --- packages/agtree/src/validator/index.ts | 86 +++++++++----------- packages/agtree/test/validator/index.test.ts | 61 +++++++------- 2 files changed, 68 insertions(+), 79 deletions(-) diff --git a/packages/agtree/src/validator/index.ts b/packages/agtree/src/validator/index.ts index 1e44d572a..c01f588be 100644 --- a/packages/agtree/src/validator/index.ts +++ b/packages/agtree/src/validator/index.ts @@ -9,12 +9,13 @@ import { getModifiersData, } from '../compatibility-tables'; import { Modifier } from '../parser/common'; +import { AdblockSyntax } from '../utils/adblockers'; import { INVALID_ERROR_PREFIX } from './constants'; const BLOCKER_PREFIX = { - ADG: 'adg_', - UBO: 'ubo_', - ABP: 'abp_', + [AdblockSyntax.Adg]: 'adg_', + [AdblockSyntax.Ubo]: 'ubo_', + [AdblockSyntax.Abp]: 'abp_', }; /** @@ -103,25 +104,35 @@ const getInvalidValidationResult = (error: string): ValidationResult => { }; /** - * Fully checks whether the given is valid for given blocker: + * Fully checks whether the given `modifier` valid for given blocker `syntax`: * is it supported by the blocker, deprecated, assignable, negatable, etc. * * @param modifiersData Parsed all modifiers data map. - * @param blockerPrefix Prefix of the blocker, e.g. 'adg_', 'ubo_', or 'abp_'. + * @param syntax Adblock syntax to check the modifier for. + * 'Common' is not supported, it should be specific — 'AdGuard', 'uBlockOrigin', or 'AdblockPlus'. * @param modifier Parsed modifier AST node. - * @param isBlocking Whether the modifier is used in blocking rule. + * @param isException Whether the modifier is used in exception rule. * Needed to check whether the modifier is allowed only in blocking or exception rules. * * @returns Result of modifier validation. */ -const validateForBlocker = ( +const validateForSpecificSyntax = ( modifiersData: ModifierDataMap, - blockerPrefix: string, + syntax: AdblockSyntax, modifier: Modifier, - isBlocking: boolean, + isException: boolean, ): ValidationResult => { + if (syntax === AdblockSyntax.Common) { + throw new Error(`Syntax should be specific, '${AdblockSyntax.Common}' is not supported`); + } + const modifierName = modifier.modifier.value; + const blockerPrefix = BLOCKER_PREFIX[syntax]; + if (!blockerPrefix) { + throw new Error(`Unknown syntax: ${syntax}`); + } + // needed for validation of negation, assignment, etc. const specificBlockerData = getSpecificBlockerData(modifiersData, blockerPrefix, modifierName); @@ -145,11 +156,11 @@ const validateForBlocker = ( }; } - if (specificBlockerData.block_only && !isBlocking) { + if (specificBlockerData.block_only && isException) { return getInvalidValidationResult(`${INVALID_ERROR_PREFIX.BLOCK_ONLY}: '${modifierName}'`); } - if (specificBlockerData.exception_only && isBlocking) { + if (specificBlockerData.exception_only && !isException) { return getInvalidValidationResult(`${INVALID_ERROR_PREFIX.EXCEPTION_ONLY}: '${modifierName}'`); } @@ -235,51 +246,28 @@ export class ModifierValidator { }; /** - * Checks whether the given modifier is valid for AdGuard. - * - * @param modifier Already parsed modifier AST node. - * @param isBlocking Whether the modifier is used in blocking rule, **default to true**. - * Needed to check whether the modifier is allowed only in blocking or exception rules. + * Checks whether the given `modifier` is valid for specified `syntax`. * - * @returns Result of modifier validation. - */ - public validateAdg = (modifier: Modifier, isBlocking = true): ValidationResult => { - if (!this.exists(modifier)) { - return getInvalidValidationResult(`${INVALID_ERROR_PREFIX.NOT_EXISTENT}: '${modifier.modifier.value}'`); - } - return validateForBlocker(this.modifiersData, BLOCKER_PREFIX.ADG, modifier, isBlocking); - }; - - /** - * Checks whether the given modifier is valid for Ublock Origin. + * For `Common` syntax it simply checks whether the modifier exists. + * For specific syntax the validation is more complex — + * deprecated, assignable, negatable and other requirements are checked. * - * @param modifier Already parsed modifier AST node. - * @param isBlocking Whether the modifier is used in blocking rule, default to true. + * @param syntax Adblock syntax to check the modifier for. + * @param modifier Modifier AST node. + * @param isException Whether the modifier is used in exception rule, default to false. * Needed to check whether the modifier is allowed only in blocking or exception rules. * * @returns Result of modifier validation. */ - public validateUbo = (modifier: Modifier, isBlocking = true): ValidationResult => { + public validate = (syntax: AdblockSyntax, modifier: Modifier, isException = false): ValidationResult => { if (!this.exists(modifier)) { return getInvalidValidationResult(`${INVALID_ERROR_PREFIX.NOT_EXISTENT}: '${modifier.modifier.value}'`); } - return validateForBlocker(this.modifiersData, BLOCKER_PREFIX.UBO, modifier, isBlocking); - }; - - /** - * Checks whether the given modifier is valid for AdBlock Plus. - * - * @param modifier Already parsed modifier AST node. - * @param isBlocking Whether the modifier is used in blocking rule, default to true. - * Needed to check whether the modifier is allowed only in blocking or exception rules. - * - * @returns Result of modifier validation. - */ - public validateAbp = (modifier: Modifier, isBlocking = true): ValidationResult => { - if (!this.exists(modifier)) { - return getInvalidValidationResult(`${INVALID_ERROR_PREFIX.NOT_EXISTENT}: '${modifier.modifier.value}'`); + // for 'Common' syntax we cannot check something more + if (syntax === AdblockSyntax.Common) { + return { ok: true }; } - return validateForBlocker(this.modifiersData, BLOCKER_PREFIX.ABP, modifier, isBlocking); + return validateForSpecificSyntax(this.modifiersData, syntax, modifier, isException); }; /** @@ -293,7 +281,7 @@ export class ModifierValidator { if (!this.exists(modifier)) { return null; } - return getBlockerDocumentationLink(this.modifiersData, BLOCKER_PREFIX.ADG, modifier); + return getBlockerDocumentationLink(this.modifiersData, BLOCKER_PREFIX[AdblockSyntax.Adg], modifier); }; /** @@ -307,7 +295,7 @@ export class ModifierValidator { if (!this.exists(modifier)) { return null; } - return getBlockerDocumentationLink(this.modifiersData, BLOCKER_PREFIX.UBO, modifier); + return getBlockerDocumentationLink(this.modifiersData, BLOCKER_PREFIX[AdblockSyntax.Ubo], modifier); }; /** @@ -321,6 +309,6 @@ export class ModifierValidator { if (!this.exists(modifier)) { return null; } - return getBlockerDocumentationLink(this.modifiersData, BLOCKER_PREFIX.ABP, modifier); + return getBlockerDocumentationLink(this.modifiersData, BLOCKER_PREFIX[AdblockSyntax.Abp], modifier); }; } diff --git a/packages/agtree/test/validator/index.test.ts b/packages/agtree/test/validator/index.test.ts index aed61cb79..c62d4872c 100644 --- a/packages/agtree/test/validator/index.test.ts +++ b/packages/agtree/test/validator/index.test.ts @@ -3,6 +3,7 @@ import { ModifierParser } from '../../src/parser/misc/modifier'; import { ModifierValidator } from '../../src/validator'; import { StringUtils } from '../../src/utils/string'; import { INVALID_ERROR_PREFIX } from '../../src/validator/constants'; +import { AdblockSyntax } from '../../src/utils/adblockers'; const DOCS_BASE_URL = { ADG: 'https://adguard.app/kb/general/ad-filtering/create-own-filters/', @@ -169,7 +170,7 @@ describe('ModifierValidator', () => { }); }); - describe('validateAdg', () => { + describe('validate for AdGuard', () => { const modifierValidator = new ModifierValidator(); describe('fully supported', () => { @@ -181,7 +182,7 @@ describe('ModifierValidator', () => { ]; test.each(supportedModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - const validationResult = modifierValidator.validateAdg(modifier); + const validationResult = modifierValidator.validate(AdblockSyntax.Adg, modifier); expect(validationResult.ok).toBeTruthy(); }); }); @@ -193,7 +194,7 @@ describe('ModifierValidator', () => { ]; test.each(supportedModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - const validationResult = modifierValidator.validateAdg(modifier); + const validationResult = modifierValidator.validate(AdblockSyntax.Adg, modifier); expect(validationResult.ok).toBeTruthy(); expect(validationResult.error).toBeUndefined(); expect(validationResult.warn?.includes('support shall be removed in the future')).toBeTruthy(); @@ -253,7 +254,7 @@ describe('ModifierValidator', () => { ]; test.each(unsupportedModifiersCases)('$actual', ({ actual, expected }) => { const modifier = getModifier(actual); - const validationResult = modifierValidator.validateAdg(modifier); + const validationResult = modifierValidator.validate(AdblockSyntax.Adg, modifier); expect(validationResult.ok).toBeFalsy(); expect(validationResult.error?.startsWith(expected)).toBeTruthy(); }); @@ -274,8 +275,8 @@ describe('ModifierValidator', () => { ]; test.each(invalidForBlockingRuleModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - // second argument is 'true' for blocking rules - const validationResult = modifierValidator.validateAdg(modifier, true); + // third argument is 'false' for blocking rules + const validationResult = modifierValidator.validate(AdblockSyntax.Adg, modifier, false); expect(validationResult.ok).toBeFalsy(); expect(validationResult.error?.startsWith(INVALID_ERROR_PREFIX.EXCEPTION_ONLY)).toBeTruthy(); }); @@ -289,8 +290,8 @@ describe('ModifierValidator', () => { ]; test.each(validForBlockingModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - // second argument is 'true' for blocking rules - const validationResult = modifierValidator.validateAdg(modifier, true); + // third argument is 'false' for blocking rules + const validationResult = modifierValidator.validate(AdblockSyntax.Adg, modifier, false); expect(validationResult.ok).toBeTruthy(); }); }); @@ -304,8 +305,8 @@ describe('ModifierValidator', () => { ]; test.each(invalidForExceptionRuleModifiers)('$actual', ({ actual, expected }) => { const modifier = getModifier(actual); - // second argument is 'false' for exception rules - const validationResult = modifierValidator.validateAdg(modifier, false); + // third argument is 'true' for exception rules + const validationResult = modifierValidator.validate(AdblockSyntax.Adg, modifier, true); expect(validationResult.ok).toBeFalsy(); expect(validationResult.error?.startsWith(expected)).toBeTruthy(); }); @@ -318,14 +319,14 @@ describe('ModifierValidator', () => { ]; test.each(validForExceptionRuleModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - // second argument is 'false' for exception rules - const validationResult = modifierValidator.validateAdg(modifier, false); + // third argument is 'true' for exception rules + const validationResult = modifierValidator.validate(AdblockSyntax.Adg, modifier, true); expect(validationResult.ok).toBeTruthy(); }); }); }); - describe('validateUbo', () => { + describe('validate for UblockOrigin', () => { const modifierValidator = new ModifierValidator(); describe('supported', () => { @@ -337,7 +338,7 @@ describe('ModifierValidator', () => { ]; test.each(supportedModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - const validationResult = modifierValidator.validateUbo(modifier); + const validationResult = modifierValidator.validate(AdblockSyntax.Ubo, modifier); expect(validationResult.ok).toBeTruthy(); }); }); @@ -395,7 +396,7 @@ describe('ModifierValidator', () => { ]; test.each(unsupportedModifiersCases)('$actual', ({ actual, expected }) => { const modifier = getModifier(actual); - const validationResult = modifierValidator.validateUbo(modifier); + const validationResult = modifierValidator.validate(AdblockSyntax.Ubo, modifier); expect(validationResult.ok).toBeFalsy(); expect(validationResult.error?.startsWith(expected)).toBeTruthy(); }); @@ -410,8 +411,8 @@ describe('ModifierValidator', () => { ]; test.each(invalidForBlockingRuleModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - // second argument is 'true' for blocking rules - const validationResult = modifierValidator.validateUbo(modifier, true); + // third argument is 'false' for blocking rules + const validationResult = modifierValidator.validate(AdblockSyntax.Ubo, modifier, false); expect(validationResult.ok).toBeFalsy(); expect(validationResult.error?.startsWith(INVALID_ERROR_PREFIX.EXCEPTION_ONLY)).toBeTruthy(); }); @@ -422,8 +423,8 @@ describe('ModifierValidator', () => { ]; test.each(validForBlockingRuleModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - // second argument is 'true' for blocking rules - const validationResult = modifierValidator.validateUbo(modifier, true); + // third argument is 'false' for blocking rules + const validationResult = modifierValidator.validate(AdblockSyntax.Ubo, modifier, false); expect(validationResult.ok).toBeTruthy(); }); }); @@ -437,8 +438,8 @@ describe('ModifierValidator', () => { ]; test.each(invalidForExceptionRuleModifiers)('$actual', ({ actual, expected }) => { const modifier = getModifier(actual); - // second argument is 'false' for exception rules - const validationResult = modifierValidator.validateUbo(modifier, false); + // third argument is 'true' for exception rules + const validationResult = modifierValidator.validate(AdblockSyntax.Ubo, modifier, true); expect(validationResult.ok).toBeFalsy(); expect(validationResult.error?.startsWith(expected)).toBeTruthy(); }); @@ -449,14 +450,14 @@ describe('ModifierValidator', () => { ]; test.each(validForExceptionRuleModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - // second argument is 'false' for exception rules - const validationResult = modifierValidator.validateUbo(modifier, false); + // third argument is 'true' for exception rules + const validationResult = modifierValidator.validate(AdblockSyntax.Ubo, modifier, true); expect(validationResult.ok).toBeTruthy(); }); }); }); - describe('validateAbp', () => { + describe('validate for AdblockPlus', () => { const modifierValidator = new ModifierValidator(); describe('supported', () => { @@ -468,7 +469,7 @@ describe('ModifierValidator', () => { ]; test.each(supportedModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - const validationResult = modifierValidator.validateAbp(modifier); + const validationResult = modifierValidator.validate(AdblockSyntax.Abp, modifier); expect(validationResult.ok).toBeTruthy(); }); }); @@ -518,7 +519,7 @@ describe('ModifierValidator', () => { ]; test.each(unsupportedModifiersCases)('$actual', ({ actual, expected }) => { const modifier = getModifier(actual); - const validationResult = modifierValidator.validateAbp(modifier); + const validationResult = modifierValidator.validate(AdblockSyntax.Abp, modifier); expect(validationResult.ok).toBeFalsy(); expect(validationResult.error?.startsWith(expected)).toBeTruthy(); }); @@ -533,8 +534,8 @@ describe('ModifierValidator', () => { test.each(invalidForBlockingRuleModifiers)('%s', (rawModifier) => { const EXPECTED_ERROR = 'Only exception rules may contain the modifier'; const modifier = getModifier(rawModifier); - // second argument is 'true' for blocking rules - const validationResult = modifierValidator.validateAbp(modifier, true); + // third argument is 'false' for blocking rules + const validationResult = modifierValidator.validate(AdblockSyntax.Abp, modifier, false); expect(validationResult.ok).toBeFalsy(); expect(validationResult.error?.startsWith(EXPECTED_ERROR)).toBeTruthy(); }); @@ -545,8 +546,8 @@ describe('ModifierValidator', () => { ]; test.each(validForBlockingRuleModifiers)('%s', (rawModifier) => { const modifier = getModifier(rawModifier); - // second argument is 'true' for blocking rules - const validationResult = modifierValidator.validateAbp(modifier, true); + // third argument is 'false' for blocking rules + const validationResult = modifierValidator.validate(AdblockSyntax.Abp, modifier, false); expect(validationResult.ok).toBeTruthy(); }); });