Skip to content

Commit

Permalink
Fix: Ignore @support block if @support or feature is not supported
Browse files Browse the repository at this point in the history
Fix #1869
  • Loading branch information
Jesus David García Gomez authored and antross committed Mar 15, 2019
1 parent 4ad0ea4 commit 0c5def3
Show file tree
Hide file tree
Showing 10 changed files with 471 additions and 58 deletions.
6 changes: 3 additions & 3 deletions packages/hint-compat-api/src/core/api-hint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { HintContext } from 'hint/dist/src/lib/hint-context';
import { IHint, Events } from 'hint/dist/src/lib/types';

import { CompatAPI, userBrowsers } from '../helpers';
import { FeatureInfo, BrowsersInfo, SupportStatementResult, ICompatLibrary } from '../types';
import { FeatureInfo, BrowsersInfo, SupportStatementResult, ICompatLibrary, TestFeatureOptions } from '../types';
import { SimpleSupportStatement, VersionValue, SupportStatement, CompatStatement, StatusBlock } from '../types-mdn.temp';
import { CompatLibraryFactory } from '../helpers/compat-library-factory';
import { browserVersions } from '../helpers/normalize-version';
Expand Down Expand Up @@ -33,7 +33,7 @@ export abstract class APIHint<T extends Events> implements IHint {
});
}

private testFeature(feature: FeatureInfo, collection: CompatStatement): boolean {
private testFeature(feature: FeatureInfo, collection: CompatStatement, options: TestFeatureOptions): boolean {
if (this.mustPackPendingReports(feature) && this.pendingReports.length > 0) {
this.generateReports();

Expand Down Expand Up @@ -66,7 +66,7 @@ export abstract class APIHint<T extends Events> implements IHint {

const hasIncompatibleBrowsers = supportStatementResult.notSupportedBrowsersCount > 0;

if (hasIncompatibleBrowsers) {
if (hasIncompatibleBrowsers && !options.skipReport) {
this.pendingReports.push([feature, supportStatementResult]);
} else {
const index = this.pendingReports.findIndex(([reportFeature]: [FeatureInfo, SupportStatementResult]) => {
Expand Down
10 changes: 6 additions & 4 deletions packages/hint-compat-api/src/helpers/compat-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { HintContext } from 'hint/dist/src/lib/hint-context';
import { Events, Event } from 'hint/dist/src/lib/types';

import { CompatFeaturesCache } from './compat-features-cache';
import { TestFeatureFunction, FeatureInfo, MDNTreeFilteredByBrowsers, ICompatLibrary } from '../types';
import { TestFeatureFunction, FeatureInfo, MDNTreeFilteredByBrowsers, ICompatLibrary, TestFeatureOptions } from '../types';
import { CompatStatement } from '../types-mdn.temp';

export abstract class CompatBase<T extends Events, K extends Event> implements ICompatLibrary {
Expand Down Expand Up @@ -44,15 +44,17 @@ export abstract class CompatBase<T extends Events, K extends Event> implements I
return this.cachedFeatures.has(feature);
}

protected checkFeatureCompatibility(feature: FeatureInfo, collection: CompatStatement | undefined): void {
protected checkFeatureCompatibility(feature: FeatureInfo, collection: CompatStatement | undefined, options: TestFeatureOptions): boolean {
if (!collection || this.isFeatureAlreadyReported(feature)) {
return;
return false;
}

const isFeatureSupported = this.testFunction(feature, collection);
const isFeatureSupported = this.testFunction(feature, collection, options);

if (!isFeatureSupported) {
this.cachedFeatures.add(feature);
}

return isFeatureSupported;
}
}
147 changes: 127 additions & 20 deletions packages/hint-compat-api/src/helpers/compat-css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@
*/

import { find } from 'lodash';
import { AtRule, Rule, Declaration, ChildNode } from 'postcss';
import { AtRule, Rule, Declaration, ChildNode, ContainerBase } from 'postcss';
import { HintContext } from 'hint/dist/src/lib/hint-context';
import { debug as d } from 'hint/dist/src/lib/utils/debug';
import { ProblemLocation } from 'hint/dist/src/lib/types';
import { StyleParse, StyleEvents } from '@hint/parser-css/dist/src/types';

import { FeatureStrategy, TestFeatureFunction, FeatureInfo, MDNTreeFilteredByBrowsers } from '../types';
import { FeatureStrategy, TestFeatureFunction, FeatureInfo, MDNTreeFilteredByBrowsers, FeatureAtSupport, TestFeatureOptions } from '../types';
import { CompatStatement } from '../types-mdn.temp';
import { CompatBase } from './compat-base';
import { evaluateQuery } from './evaluate-query';

const debug: debug.IDebugger = d(__filename);

Expand All @@ -24,13 +25,8 @@ export class CompatCSS extends CompatBase<StyleEvents, StyleParse> {
hintContext.on('parse::end::css', this.onParse.bind(this));
}

public searchFeatures(parse: StyleParse) {
parse.ast.walk((node: ChildNode) => {
const strategy = this.chooseStrategyToSearchCSSFeature(node);
const location = this.getProblemLocationFromNode(node);

strategy.testFeature(node, location);
});
public searchFeatures(parse: StyleParse): void {
this.walk(parse.ast);
}

private onParse(parse: StyleParse) {
Expand All @@ -41,14 +37,123 @@ export class CompatCSS extends CompatBase<StyleEvents, StyleParse> {
this.searchFeatures(parse);
}

private testFeature(strategyName: string, featureNameWithPrefix: string, location?: ProblemLocation, subfeatureNameWithPrefix?: string): void {
private getSupportFeature(featureString: string) {
const featureRegex = /([^:]+):([^)]*)/g;
const exec = featureRegex.exec(featureString);

if (!exec) {
return null;
}

return {
property: exec[1],
value: exec[2]
};
}

/**
* Transform the condition of a @support block to an array of {prop: string, value: string}
* e.g. (display: table-cell) and ((display: list-item) and (display:run-in))
*/
private getSupportFeatures(conditionsString: string): (FeatureAtSupport | null)[] {
// Ignore selector().
const conditionRegex = /(?<!selector)\(([^()]+)\)/gi;

const conditions = [];

let exec = conditionRegex.exec(conditionsString);

while (exec) {
conditions.push(exec[1]);

exec = conditionRegex.exec(conditionsString);
}

return conditions.map(this.getSupportFeature);
}

private validateSupportFeatures(params: string, location: ProblemLocation | undefined): boolean {
const features = this.getSupportFeatures(params);
// Ignore selector(...)
let query = params;

for (const feature of features) {
if (!feature) {
continue;
}

const featureNode = {
prop: feature.property.trim(),
type: 'decl',
value: feature.value.trim()
} as ChildNode;

const featureStrategy = this.chooseStrategyToSearchCSSFeature(featureNode);
const featureSupported = featureStrategy.testFeature(featureNode, location, { skipReport: true });

query = query.replace(`(${feature.property}:${feature.value})`, featureSupported.toString());
}

// Remove empty spaces between selector/not and the next statement
query = query.replace(/selector\s*/g, 'selector');
query = query.replace(/not\s*/gi, 'not');
const valid = evaluateQuery(query);

return valid;
}

/**
* Walk through the nodes.
*
* Using a custom method instead of ast.walk() because
* if the browser doesn't support @support or it considition,
* we have to skip the node and it children.
*/
private walk(ast: ContainerBase) {
const nodes: ChildNode[] | undefined = ast.nodes;

if (!nodes) {
return;
}

for (const node of nodes) {
const strategy = this.chooseStrategyToSearchCSSFeature(node);
const location = this.getProblemLocationFromNode(node);

if (node.type === 'atrule' && node.name === 'supports') {
const supported = strategy.testFeature(node, location, { skipReport: true });

// If browser doesn't support @support ignore the @support block.
if (!supported) {
continue;
}

const valid = this.validateSupportFeatures(node.params, location);

// At least one feature is not supported.
if (!valid) {
continue;
}

this.walk(node as ContainerBase);

continue;
}

strategy.testFeature(node, location);

this.walk(node as ContainerBase);
}
}

private testFeature(strategyName: string, featureNameWithPrefix: string, location?: ProblemLocation, subfeatureNameWithPrefix?: string, options: TestFeatureOptions = {}): boolean {
const collection: CompatStatement | undefined = this.MDNData[strategyName];

if (!collection) {
// Review: Throw an error
debug('Error: The strategy does not exist.');

return;
return false;
}

const [prefix, name] = this.getPrefix(featureNameWithPrefix);
Expand All @@ -60,7 +165,7 @@ export class CompatCSS extends CompatBase<StyleEvents, StyleParse> {
feature.subFeature = { displayableName: name, name, prefix };
}

this.checkFeatureCompatibility(feature, collection);
return this.checkFeatureCompatibility(feature, collection, options);
}

private getProblemLocationFromNode(node: ChildNode): ProblemLocation | undefined {
Expand All @@ -82,8 +187,8 @@ export class CompatCSS extends CompatBase<StyleEvents, StyleParse> {
return node.type === 'atrule';
},

testFeature: (node: AtRule, location) => {
this.testFeature('at-rules', node.name, location);
testFeature: (node: AtRule, location?: ProblemLocation, options?: TestFeatureOptions) => {
return this.testFeature('at-rules', node.name, location, undefined, options);
}
};

Expand All @@ -92,8 +197,8 @@ export class CompatCSS extends CompatBase<StyleEvents, StyleParse> {
return node.type === 'rule';
},

testFeature: (node: Rule, location) => {
this.testFeature('selectors', node.selector, location);
testFeature: (node: Rule, location?: ProblemLocation, options?: TestFeatureOptions) => {
return this.testFeature('selectors', node.selector, location, undefined, options);
}
};

Expand All @@ -102,9 +207,9 @@ export class CompatCSS extends CompatBase<StyleEvents, StyleParse> {
return node.type === 'decl';
},

testFeature: (node: Declaration, location) => {
this.testFeature('properties', node.prop, location);
this.testFeature('properties', node.prop, location, node.value);
testFeature: (node: Declaration, location?: ProblemLocation, options?: TestFeatureOptions) => {
return this.testFeature('properties', node.prop, location, undefined, options) &&
this.testFeature('properties', node.prop, location, node.value, options);
}
};

Expand All @@ -113,7 +218,9 @@ export class CompatCSS extends CompatBase<StyleEvents, StyleParse> {
return true;
},

testFeature: () => { }
testFeature: () => {
return false;
}
};

const strategies = {
Expand Down
2 changes: 1 addition & 1 deletion packages/hint-compat-api/src/helpers/compat-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class CompatHTML extends CompatBase<Events, Event> {
}

private testFeature(collection: CompatStatement | undefined, feature: FeatureInfo): void {
this.checkFeatureCompatibility(feature, collection);
this.checkFeatureCompatibility(feature, collection, { skipReport: false });
}

private walk(callback: (element: ElementFound) => any) {
Expand Down
113 changes: 113 additions & 0 deletions packages/hint-compat-api/src/helpers/evaluate-query.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
enum ValidStatements {
and = 'and',
empty = '',
false = 'false',
negation = 'not',
or = 'or',
selector = 'selector',
true = 'true'
}

const validStatementsStrings: string[] = Object.values(ValidStatements);

// nottrue
validStatementsStrings.push(`${ValidStatements.negation}${ValidStatements.true}`);
// notfalse
validStatementsStrings.push(`${ValidStatements.negation}${ValidStatements.false}`);
// selectortrue
validStatementsStrings.push(`${ValidStatements.selector}${ValidStatements.true}`);
// selectorfalse
validStatementsStrings.push(`${ValidStatements.selector}${ValidStatements.false}`);

export const evaluateQuery = (queryString: string): boolean => {
const query = queryString.toLowerCase();

if (query === 'false') {
return false;
} else if (query === 'true') {
return true;
}

const regex = /\(([^()]*)\)/;
/*
* Get content inside parentheses.
*/
let exec = regex.exec(query);
let finalQuery = query;

while (exec !== null) {
const condition = exec[1];

finalQuery = finalQuery.replace(exec[0], evaluateQuery(condition).toString());

exec = regex.exec(finalQuery);
}

/*
* Split the query in statements.
*/
const splitQuery = finalQuery.trim().split(' ');
let operator: string | null = null;
let result: boolean | null = null;

for (const partialQuery of splitQuery) {
/*
* We expect only the strings in validStatementsStrings.
* If we receive something diferente, validate the whole
* query as true.
* e.g. var, x, =, console, etc.
*/
if (!partialQuery.startsWith(ValidStatements.negation) && !partialQuery.startsWith(ValidStatements.selector) && !validStatementsStrings.includes(partialQuery)) {
return true;
}

/*
* If partialQuery is an empty string, ignore it.
*/
if (!partialQuery) {
continue;
}

/*
* Store the operator statement for the next item.
*/
if (partialQuery === ValidStatements.and || partialQuery === ValidStatements.or) {
operator = partialQuery;

continue;
}

/*
* Check if the statement is negated, is a selector, or neither of those.
*/
let partialResult: boolean;

if (partialQuery.startsWith(ValidStatements.negation)) {
partialResult = !evaluateQuery(partialQuery.substr(ValidStatements.negation.length));
} else if (partialQuery.startsWith(ValidStatements.selector)) {
partialResult = true;
} else {
partialResult = evaluateQuery(partialQuery);
}

/*
* This should be true only in the first iteration.
*/
if (result === null) {
result = partialResult;

continue;
}

if (operator) {
/*
* Calculate the accumulated result.
*/
result = operator === ValidStatements.and ? result && partialResult : result || partialResult;

operator = null;
}
}

return !!result;
};
Loading

0 comments on commit 0c5def3

Please sign in to comment.