Permalink
Browse files

Breaking: Change `context.report` to take an `options` object

Now `report` only has two required parameters, a string `resource`
and a string `message`. All other parameters are now optionally passed
as part of a third `options` parameter of type `ReportOptions`.

Old code like:

```ts
await context.report(
    resource,
    null,
    message,
    undefined,
    location
);
```

Can now be written as:

```ts
await context.report( resource, message, { location });
```

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

Fix webhintio/rfcs#47
Close #1415
  • Loading branch information...
antross authored and alrra committed Oct 17, 2018
1 parent b7246da commit 0e82bcad9bd5fb3626bf68d94278b89d685b46c7
Showing with 327 additions and 187 deletions.
  1. +2 βˆ’2 packages/create-hint/src/templates/partial-event-code.hbs
  2. +6 βˆ’1 packages/hint-amp-validator/src/hint.ts
  3. +32 βˆ’11 packages/hint-apple-touch-icons/src/hint.ts
  4. +4 βˆ’2 packages/hint-axe/src/hint.ts
  5. +2 βˆ’2 packages/hint-babel-config/src/is-valid.ts
  6. +6 βˆ’6 packages/hint-content-type/src/hint.ts
  7. +3 βˆ’1 packages/hint-disown-opener/src/hint.ts
  8. +3 βˆ’3 packages/hint-doctype/src/hint.ts
  9. +17 βˆ’9 packages/hint-highest-available-document-mode/src/hint.ts
  10. +6 βˆ’2 packages/hint-html-checker/src/hint.ts
  11. +8 βˆ’8 packages/hint-http-cache/src/hint.ts
  12. +28 βˆ’26 packages/hint-http-compression/src/hint.ts
  13. +4 βˆ’4 packages/hint-https-only/src/hint.ts
  14. +3 βˆ’3 packages/hint-image-optimization-cloudinary/src/hint.ts
  15. +9 βˆ’3 packages/hint-manifest-app-name/src/hint.ts
  16. +4 βˆ’4 packages/hint-manifest-exists/src/hint.ts
  17. +3 βˆ’1 packages/hint-manifest-file-extension/src/hint.ts
  18. +14 βˆ’5 packages/hint-manifest-is-valid/src/hint.ts
  19. +7 βˆ’7 packages/hint-meta-charset-utf-8/src/hint.ts
  20. +12 βˆ’6 packages/hint-meta-theme-color/src/hint.ts
  21. +22 βˆ’9 packages/hint-meta-viewport/src/hint.ts
  22. +1 βˆ’1 packages/hint-minified-js/src/hint.ts
  23. +2 βˆ’2 packages/hint-no-bom/src/hint.ts
  24. +7 βˆ’5 packages/hint-no-broken-links/src/hint.ts
  25. +6 βˆ’2 packages/hint-no-disallowed-headers/src/hint.ts
  26. +1 βˆ’1 packages/hint-no-friendly-error-pages/src/hint.ts
  27. +3 βˆ’1 packages/hint-no-html-only-headers/src/hint.ts
  28. +3 βˆ’1 packages/hint-no-http-redirects/src/hint.ts
  29. +3 βˆ’3 packages/hint-no-p3p/src/hint.ts
  30. +3 βˆ’1 packages/hint-no-protocol-relative-urls/src/hint.ts
  31. +4 βˆ’2 packages/hint-no-vulnerable-javascript-libraries/src/hint.ts
  32. +1 βˆ’1 packages/hint-performance-budget/src/hint.ts
  33. +20 βˆ’8 packages/hint-sri/src/hint.ts
  34. +5 βˆ’5 packages/hint-ssllabs/src/hint.ts
  35. +15 βˆ’9 packages/hint-strict-transport-security/src/hint.ts
  36. +3 βˆ’3 packages/hint-stylesheet-limits/src/hint.ts
  37. +1 βˆ’1 packages/hint-typescript-config/src/helpers/config-checker.ts
  38. +1 βˆ’1 packages/hint-typescript-config/src/import-helpers.ts
  39. +2 βˆ’2 packages/hint-typescript-config/src/is-valid.ts
  40. +4 βˆ’1 packages/hint-typescript-config/src/target.ts
  41. +2 βˆ’2 packages/hint-validate-set-cookie-header/src/hint.ts
  42. +1 βˆ’1 packages/hint-webpack-config/src/config-exists.ts
  43. +1 βˆ’1 packages/hint-webpack-config/src/is-installed.ts
  44. +1 βˆ’1 packages/hint-webpack-config/src/is-valid.ts
  45. +3 βˆ’3 packages/hint-webpack-config/src/module-esnext-typescript.ts
  46. +3 βˆ’3 packages/hint-webpack-config/src/modules-false-babel.ts
  47. +1 βˆ’1 packages/hint-webpack-config/src/no-devtool-in-prod.ts
  48. +3 βˆ’3 packages/hint-x-content-type-options/src/hint.ts
  49. +3 βˆ’1 packages/hint/docs/contributor-guide/guides/create-custom-hint.md
  50. +14 βˆ’4 packages/hint/docs/contributor-guide/how-to/hint.md
  51. +15 βˆ’2 packages/hint/src/lib/hint-context.ts
@@ -5,12 +5,12 @@ debug(`Validating hint {{name}}`);
/*
* This is where all the magic happens. Any errors found should be
* reported using the `context` object. E.g.:
* await context.report(resource, null, 'Add error message here.');
* await context.report(resource, 'Add error message here.');
*
* More information on how to develop a hint is available in:
* https://webhint.io/docs/contributor-guide/hints/
*/
if (Math.ceil(Math.random()) === 0) {
await context.report(resource, null, 'Add error message here.');
await context.report(resource, 'Add error message here.');
}
@@ -80,7 +80,12 @@ export default class AmpValidatorHint implements IHint {
if (errorsOnly && error.severity !== 'ERROR') {
debug(`AMP error doesn't meet threshold for reporting`);
} else {
await context.report(resource, null, message, '', { column: error.col, line: error.line });
const location = {
column: error.col,
line: error.line
};
await context.report(resource, message, { location });
}
}
}
@@ -85,7 +85,9 @@ export default class AppleTouchIconsHint implements IHint {
*/
if (!appleTouchIconHref) {
await context.report(resource, appleTouchIcon, `'apple-touch-icon' link element should have non-empty 'href' attribute.`);
const message = `'apple-touch-icon' link element should have non-empty 'href' attribute.`;
await context.report(resource, message, { element: appleTouchIcon });
return;
}
@@ -122,15 +124,20 @@ export default class AppleTouchIconsHint implements IHint {
networkData = await context.fetchContent(appleTouchIconURL);
} catch (e) {
debug(`Failed to fetch the ${appleTouchIconHref} file`);
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' could not be fetched (request failed).`);
const message = `'${appleTouchIconHref}' could not be fetched (request failed).`;
await context.report(resource, message, { element: appleTouchIcon });
return;
}
const response = networkData.response;
if (response.statusCode !== 200) {
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' could not be fetched (status code: ${response.statusCode}).`);
const message = `'${appleTouchIconHref}' could not be fetched (status code: ${response.statusCode}).`;
await context.report(resource, message, { element: appleTouchIcon });
return;
}
@@ -156,7 +163,9 @@ export default class AppleTouchIconsHint implements IHint {
image = getImageData(response.body.rawContent);
} catch (e) {
if (e instanceof TypeError) {
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' should be a valid PNG image.`);
const message = `'${appleTouchIconHref}' should be a valid PNG image.`;
await context.report(resource, message, { element: appleTouchIcon });
} else {
debug(`'getImageData' failed for '${appleTouchIconURL}'`);
}
@@ -167,13 +176,17 @@ export default class AppleTouchIconsHint implements IHint {
// Check if the image is a PNG.
if (image.type !== 'png') {
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' should be a PNG image.`);
const message = `'${appleTouchIconHref}' should be a PNG image.`;
await context.report(resource, message, { element: appleTouchIcon });
}
// Check if the image is 180x180px.
if (image.width !== 180 || image.height !== 180) {
await context.report(resource, appleTouchIcon, `'${appleTouchIconHref}' should be 180x180px.`);
const message = `'${appleTouchIconHref}' should be 180x180px.`;
await context.report(resource, message, { element: appleTouchIcon });
}
// TODO: Check if the image has some kind of transparency.
@@ -220,7 +233,7 @@ export default class AppleTouchIconsHint implements IHint {
const appleTouchIcons: IAsyncHTMLElement[] = getAppleTouchIcons(await pageDOM.querySelectorAll('link'));
if (appleTouchIcons.length === 0) {
await context.report(resource, null, `'apple-touch-icon' link element was not specified.`);
await context.report(resource, `'apple-touch-icon' link element was not specified.`);
return;
}
@@ -238,7 +251,9 @@ export default class AppleTouchIconsHint implements IHint {
*/
if (normalizeString(appleTouchIcon.getAttribute('rel')) !== 'apple-touch-icon') {
await context.report(resource, appleTouchIcon, `'apple-touch-icon' link element should have 'rel="apple-touch-icon".`);
const message = `'apple-touch-icon' link element should have 'rel="apple-touch-icon".`;
await context.report(resource, message, { element: appleTouchIcon });
}
/*
@@ -251,7 +266,9 @@ export default class AppleTouchIconsHint implements IHint {
*/
if (appleTouchIcon.getAttribute('sizes')) {
await context.report(resource, appleTouchIcon, `'apple-touch-icon' link element should not have 'sizes' attribute.`);
const message = `'apple-touch-icon' link element should not have 'sizes' attribute.`;
await context.report(resource, message, { element: appleTouchIcon });
}
/*
@@ -269,7 +286,9 @@ export default class AppleTouchIconsHint implements IHint {
for (const icon of bodyAppleTouchIcons) {
if (icon.isSame(appleTouchIcon)) {
await context.report(resource, appleTouchIcon, `'apple-touch-icon' link element should be specified in the '<head>'.`);
const message = `'apple-touch-icon' link element should be specified in the '<head>'.`;
await context.report(resource, message, { element: appleTouchIcon });
}
}
@@ -279,7 +298,9 @@ export default class AppleTouchIconsHint implements IHint {
for (const icon of appleTouchIcons) {
if (!icon.isSame(appleTouchIcon)) {
await context.report(resource, icon, `'apple-touch-icon' link element is not needed as one was already specified.`);
const message = `'apple-touch-icon' link element is not needed as one was already specified.`;
await context.report(resource, message, { element: icon });
}
}
};
@@ -124,7 +124,9 @@ export default class AxeHint implements IHint {
message = `Error executing script: '${e.message}'`;
}
await context.report(resource, null, `${message}. Please try again later, or report an issue if this problem persists.`, undefined, undefined, Severity.warning);
message = `${message}. Please try again later, or report an issue if this problem persists.`;
await context.report(resource, message, { severity: Severity.warning });
debug('Error executing script %O', e);
return;
@@ -149,7 +151,7 @@ export default class AxeHint implements IHint {
const element = await getElement(node);
// TODO: find the right element here using node.target[0] ?
await context.report(resource, element, violation.help);
await context.report(resource, violation.help, { element });
return;
});
@@ -33,7 +33,7 @@ export default class BabelConfigIsValidHint implements IHint {
debug(`${event} received`);
await context.report(resource, null, error.message);
await context.report(resource, error.message);
};
const invalidSchema = async (fetchEnd: BabelConfigInvalidSchema) => {
@@ -45,7 +45,7 @@ export default class BabelConfigIsValidHint implements IHint {
const message = prettifiedErrors[i];
const location = errors[i].location;
await context.report(resource, null, message, undefined, location);
await context.report(resource, message, { location });
}
};
@@ -85,7 +85,7 @@ export default class ContentTypeHint implements IHint {
// Check if the `Content-Type` header was sent.
if (contentTypeHeaderValue === null) {
await context.report(resource, element, `Response should include 'content-type' header.`);
await context.report(resource, `Response should include 'content-type' header.`, { element });
return;
}
@@ -99,7 +99,7 @@ export default class ContentTypeHint implements IHint {
if (userDefinedMediaType) {
if (normalizeString(userDefinedMediaType) !== contentTypeHeaderValue) {
await context.report(resource, element, `'content-type' header value should be '${userDefinedMediaType}'.`);
await context.report(resource, `'content-type' header value should be '${userDefinedMediaType}'.`, { element });
}
return;
@@ -116,7 +116,7 @@ export default class ContentTypeHint implements IHint {
contentType = parse(contentTypeHeaderValue);
} catch (e) {
await context.report(resource, element, `'content-type' header value should be valid (${e.message}).`);
await context.report(resource, `'content-type' header value should be valid (${e.message}).`, { element });
return;
}
@@ -145,21 +145,21 @@ export default class ContentTypeHint implements IHint {
// * media type
if (mediaType && (mediaType !== originalMediaType)) {
await context.report(resource, element, `'content-type' header media type value should be '${mediaType}', not '${originalMediaType}'.`);
await context.report(resource, `'content-type' header media type value should be '${mediaType}', not '${originalMediaType}'.`, { element });
}
// * charset value
if (charset) {
if (!originalCharset || (charset !== originalCharset)) {
await context.report(resource, element, `'content-type' header charset value should be '${charset}'${originalCharset ? `, not '${originalCharset}'` : ''}.`);
await context.report(resource, `'content-type' header charset value should be '${charset}'${originalCharset ? `, not '${originalCharset}'` : ''}.`, { element });
}
} else if (originalCharset &&
![
'text/html',
'application/xhtml+xml'
].includes(originalMediaType)) {
await context.report(resource, element, `'content-type' header value should not contain 'charset=${originalCharset}'.`);
await context.report(resource, `'content-type' header value should not contain 'charset=${originalCharset}'.`, { element });
}
};
@@ -64,7 +64,9 @@ export default class DisownOpenerHint implements IHint {
});
if (requiredValues.length !== 0) {
await context.report(resource, element, `'${cutString(await element.outerHTML(), 100)}' should have 'rel' attribute value include ${prettyPrintArray(requiredValues)} ${requiredValues.length === 1 ? 'keyword' : 'keywords'}.`, hrefValue);
const message = `'${cutString(await element.outerHTML(), 100)}' should have 'rel' attribute value include ${prettyPrintArray(requiredValues)} ${requiredValues.length === 1 ? 'keyword' : 'keywords'}.`;
await context.report(resource, message, { content: hrefValue, element });
}
};
@@ -37,8 +37,8 @@ export default class implements IHint {
line: 0
};
const report = async (resource: string, message: string, problemLocation = defaultProblemLocation): Promise<void> => {
await context.report(resource, null, message, undefined, problemLocation);
const report = async (resource: string, message: string, location = defaultProblemLocation): Promise<void> => {
await context.report(resource, message, { location });
};
const getCurrentDoctypeProblemLocation = (text: string): ProblemLocation[] => {
@@ -103,7 +103,7 @@ export default class implements IHint {
const { resource, response } = fetchEnd;
if (!response || !response.body || !response.body.content) {
await context.report(resource, null, 'Resource has no content.', undefined);
await context.report(resource, 'Resource has no content.');
return;
}
@@ -72,7 +72,7 @@ export default class HighestAvailableDocumentModeHint implements IHint {
*/
if (!requireMetaElement && !suggestRemoval) {
await context.report(resource, null, `Response should include 'x-ua-compatible' header.`);
await context.report(resource, `Response should include 'x-ua-compatible' header.`);
}
return;
@@ -85,13 +85,13 @@ export default class HighestAvailableDocumentModeHint implements IHint {
*/
if (suggestRemoval) {
await context.report(resource, null, `Response should not include unneeded 'x-ua-compatible' header.`);
await context.report(resource, `Response should not include unneeded 'x-ua-compatible' header.`);
return;
}
if (headerValue !== 'ie=edge') {
await context.report(resource, null, `'x-ua-compatible' header value should be 'ie=edge', not '${!originalHeaderValue ? '' : originalHeaderValue}'.`);
await context.report(resource, `'x-ua-compatible' header value should be 'ie=edge', not '${!originalHeaderValue ? '' : originalHeaderValue}'.`);
}
/*
@@ -122,7 +122,7 @@ export default class HighestAvailableDocumentModeHint implements IHint {
`'x-ua-compatible' meta element should not be specified, and instead, equivalent HTTP header should be used.`;
for (const metaElement of XUACompatibleMetaElements) {
await context.report(resource, metaElement, errorMessage);
await context.report(resource, errorMessage, { element: metaElement });
}
}
@@ -132,7 +132,7 @@ export default class HighestAvailableDocumentModeHint implements IHint {
// If the user requested the meta element to be specified.
if (XUACompatibleMetaElements.length === 0) {
await context.report(resource, null, `'x-ua-compatible' meta element should be specified.`);
await context.report(resource, `'x-ua-compatible' meta element should be specified.`);
return;
}
@@ -148,7 +148,9 @@ export default class HighestAvailableDocumentModeHint implements IHint {
// * it has the value `ie=edge`.
if (normalizeString(contentValue) !== 'ie=edge') {
await context.report(resource, XUACompatibleMetaElement, `'x-ua-compatible' meta element 'content' attribute value should be 'ie=edge', not '${!contentValue ? '' : contentValue}'.`);
const message = `'x-ua-compatible' meta element 'content' attribute value should be 'ie=edge', not '${!contentValue ? '' : contentValue}'.`;
await context.report(resource, message, { element: XUACompatibleMetaElement });
}
/*
@@ -165,7 +167,9 @@ export default class HighestAvailableDocumentModeHint implements IHint {
for (const headElement of headElements) {
if (headElement.isSame(XUACompatibleMetaElement)) {
if (!metaElementIsBeforeRequiredElements) {
await context.report(resource, XUACompatibleMetaElement, `'x-ua-compatible' meta element should be specified before all other elements except for '<title>' and other '<meta>' elements.`);
const message = `'x-ua-compatible' meta element should be specified before all other elements except for '<title>' and other '<meta>' elements.`;
await context.report(resource, message, { element: XUACompatibleMetaElement });
}
break;
@@ -181,7 +185,9 @@ export default class HighestAvailableDocumentModeHint implements IHint {
const bodyMetaElements: IAsyncHTMLElement[] = getXUACompatibleMetaElements(await pageDOM.querySelectorAll('body meta'));
if ((bodyMetaElements.length > 0) && bodyMetaElements[0].isSame(XUACompatibleMetaElement)) {
await context.report(resource, XUACompatibleMetaElement, `'x-ua-compatible' meta element should be specified in the '<head>', not '<body>'.`);
const message = `'x-ua-compatible' meta element should be specified in the '<head>', not '<body>'.`;
await context.report(resource, message, { element: XUACompatibleMetaElement });
return;
}
@@ -192,7 +198,9 @@ export default class HighestAvailableDocumentModeHint implements IHint {
const metaElements = XUACompatibleMetaElements.slice(1);
for (const metaElement of metaElements) {
await context.report(resource, metaElement, `'x-ua-compatible' meta element is not needed as one was already specified.`);
const message = `'x-ua-compatible' meta element is not needed as one was already specified.`;
await context.report(resource, message, { element: metaElement });
}
}
};
@@ -121,13 +121,17 @@ export default class HtmlCheckerHint implements IHint {
line: messageItem.lastLine
};
return context.report(resource, null, messageItem.message, undefined, position, Severity[messageItem.subType], messageItem.extract);
return context.report(resource, messageItem.message, {
codeSnippet: messageItem.extract,
location: position,
severity: Severity[messageItem.subType]
});
};
};
const notifyError = async (resource: string, error: any) => {
debug(`Error getting HTML checker result for ${resource}.`, error);
await context.report(resource, null, `Could not get results from HTML checker for '${resource}'. Error: '${error}'.`);
await context.report(resource, `Could not get results from HTML checker for '${resource}'. Error: '${error}'.`);
};
const requestRetry = async (options: OptionsWithUrl, retries: number = 3): Promise<any> => {
Oops, something went wrong.

0 comments on commit 0e82bca

Please sign in to comment.