Skip to content

Commit

Permalink
update ModifierValidator for AGLint needs. AG-23003
Browse files Browse the repository at this point in the history 
Squashed commit of the following:

commit 7beb82a
Author: David Tota <d.tota@adguard.com>
Date:   Thu Jul 20 12:36:51 2023 +0300

    fix error message

commit 84a01f5
Merge: 7dd79b1 8b9101e
Author: Slava Leleka <v.leleka@adguard.com>
Date:   Wed Jul 19 18:22:35 2023 +0300

    Merge branch 'master' into fix/AG-23003-04

commit 7dd79b1
Merge: 8730bdb cb73a90
Author: Slava Leleka <v.leleka@adguard.com>
Date:   Wed Jul 19 14:56:18 2023 +0300

    Merge branch 'master' into fix/AG-23003-04

commit 8730bdb
Author: Slava Leleka <v.leleka@adguard.com>
Date:   Wed Jul 19 14:30:37 2023 +0300

    update ModifierValidator for AGLint needs
  • Loading branch information
@slavaleleka
slavaleleka committed Jul 20, 2023
1 parent 8b9101e commit b716498
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 79 deletions.
86 changes: 37 additions & 49 deletions packages/agtree/src/validator/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_',
};

/**
Expand Down Expand Up @@ -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);

Expand All @@ -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}'`);
}

Expand Down Expand Up @@ -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);
};

/**
Expand All @@ -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);
};

/**
Expand All @@ -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);
};

/**
Expand All @@ -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);
};
}
61 changes: 31 additions & 30 deletions packages/agtree/test/validator/index.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/',
Expand Down Expand Up @@ -169,7 +170,7 @@ describe('ModifierValidator', () => {
});
});

describe('validateAdg', () => {
describe('validate for AdGuard', () => {
const modifierValidator = new ModifierValidator();

describe('fully supported', () => {
Expand All @@ -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();
});
});
Expand All @@ -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();
Expand Down Expand Up @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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();
});
});
Expand All @@ -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();
});
Expand All @@ -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', () => {
Expand All @@ -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();
});
});
Expand Down Expand Up @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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();
});
});
Expand All @@ -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();
});
Expand All @@ -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', () => {
Expand All @@ -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();
});
});
Expand Down Expand Up @@ -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();
});
Expand All @@ -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();
});
Expand All @@ -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();
});
});
Expand Down

0 comments on commit b716498

Please sign in to comment.