Skip to content

Commit

Permalink
Should also report errors by default (#75)
Browse files Browse the repository at this point in the history 
* Should also report errors by default

* Fix tests

* Add formatting for errors, auto throw on errors
  • Loading branch information
@onigoetz
onigoetz authored Jan 16, 2024
1 parent af4f833 commit 7109853
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 24 deletions.
14 changes: 6 additions & 8 deletions lib/formatter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,6 @@ var path = require('path');
var firstBy = require('thenby');
var util = require('./util');

var supportsLargeCharset =
process.platform !== 'win32' ||
process.env.CI ||
process.env.TERM === 'xterm-256color';
var warningSymbol = supportsLargeCharset ? '⚠' : '!!';

function createSortFunction(positionless, sortByPosition) {
var positionValue = 0

Expand Down Expand Up @@ -76,8 +70,12 @@ module.exports = function (opts) {
str += '\t';
}

if (!options.noIcon && message.type === 'warning') {
str += pico.yellow(warningSymbol + ' ');
if (!options.noIcon) {
if (message.type === 'warning') {
str += pico.yellow(util.warningSymbol + ' ');
} else if (message.type === 'error') {
str += pico.red(util.errorSymbol + ' ');
}
}

str += message.text;
Expand Down
26 changes: 18 additions & 8 deletions lib/reporter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module.exports = function (opts = {}) {
};
}

var messageFilter = opts.filter || ((message) => message.type === 'warning');
var messageFilter = opts.filter || ((message) => message.type === 'warning' || message.type === 'error');

return {
postcssPlugin: 'postcss-reporter',
Expand All @@ -45,13 +45,22 @@ module.exports = function (opts = {}) {
? ''
: result.root.source.input.file || result.root.source.input.id;

let errorCount = 0;
let warningCount = 0;

var sourceGroupedMessages = messagesToLog.reduce((grouped, message) => {
const key = util.getLocation(message).file || resultSource;

if (!grouped.hasOwnProperty(key)) {
grouped[key] = [];
}

if (message.type === 'error') {
errorCount++;
} else if (message.type === 'warning') {
warningCount++;
}

grouped[key].push(message);

return grouped;
Expand All @@ -78,9 +87,15 @@ module.exports = function (opts = {}) {

if (!report) return;

const summaryColor = errorCount > 0 ? 'red' : 'yellow';
const summarySymbol = errorCount > 0 ? util.errorSymbol : util.warningSymbol;
const summary = `${summarySymbol} ${messagesToLog.length} ${util.plur('problem', messagesToLog.length)} (${errorCount} ${util.plur('error')}, ${warningCount} ${util.plur('warning')})`

report += `\n ${pico[summaryColor](pico.bold(summary))}\n`;

console.log(report);

if (opts.throwError && shouldThrowError()) {
if (shouldThrowError()) {
throw new Error(
pico.red(
pico.bold('\n** postcss-reporter: warnings or errors were found **')
Expand All @@ -89,12 +104,7 @@ module.exports = function (opts = {}) {
}

function shouldThrowError() {
return (
messagesToLog.length &&
messagesToLog.some((message) => {
return message.type === 'warning' || message.type === 'error';
})
);
return opts.throwError || errorCount > 0;
}
},
};
Expand Down
12 changes: 12 additions & 0 deletions lib/util.js
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
var supportsLargeCharset =
process.platform !== 'win32' ||
process.env.CI ||
process.env.TERM === 'xterm-256color';

exports.getLocation = function (message) {
var messageNode = message.node;

Expand All @@ -17,3 +22,10 @@ exports.getLocation = function (message) {
location.file = messageInput.file || messageInput.id;
return location;
};

exports.plur = function plur(word, count) {
return (count === 1 ? word : `${word}s`);
}

exports.warningSymbol = supportsLargeCharset ? '⚠' : '!!';
exports.errorSymbol = supportsLargeCharset ? '✖' : 'xx';
11 changes: 6 additions & 5 deletions test/formatter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ var stripColor = require('strip-color');
var defaultFormatter = formatter();

var colorlessWarning = '⚠';
var colorlessError = '✖';

var basicMessages = [
{
Expand Down Expand Up @@ -36,7 +37,7 @@ var basicOutput = '\n<input css 1>' +
'\n' + colorlessWarning + ' foo warning [foo]' +
'\n' + colorlessWarning + ' bar warning [bar]' +
'\n' + colorlessWarning + ' baz warning [baz]' +
'\nbaz error [baz]' +
'\n' + colorlessError + ' baz error [baz]' +
'\n';

var basicOutputMinimal = '\n<input css 1>' +
Expand Down Expand Up @@ -104,14 +105,14 @@ var complexMessages = [
];

var complexOutput = '\nstyle/rainbows/horses.css' +
'\nbaz error [baz]' +
'\n' + colorlessError + ' baz error [baz]' +
'\n1:99\t' + colorlessWarning + ' bar warning [bar]' +
'\n3:5\t' + colorlessWarning + ' foo warning [foo]' +
'\n8:13\t' + colorlessWarning + ' ha warning [foo]' +
'\n';

var noPositionSortOutput = '\nstyle/rainbows/horses.css' +
'\nbaz error [baz]' +
'\n' + colorlessError + ' baz error [baz]' +
'\n3:5\t' + colorlessWarning + ' foo warning [foo]' +
'\n1:99\t' + colorlessWarning + ' bar warning [bar]' +
'\n8:13\t' + colorlessWarning + ' ha warning [foo]' +
Expand All @@ -121,12 +122,12 @@ var positionlessLastOutput = '\nstyle/rainbows/horses.css' +
'\n1:99\t' + colorlessWarning + ' bar warning [bar]' +
'\n3:5\t' + colorlessWarning + ' foo warning [foo]' +
'\n8:13\t' + colorlessWarning + ' ha warning [foo]' +
'\nbaz error [baz]' +
'\n' + colorlessError + ' baz error [baz]' +
'\n';

var noSortOutput = '\nstyle/rainbows/horses.css' +
'\n3:5\t' + colorlessWarning + ' foo warning [foo]' +
'\nbaz error [baz]' +
'\n' + colorlessError + ' baz error [baz]' +
'\n1:99\t' + colorlessWarning + ' bar warning [bar]' +
'\n8:13\t' + colorlessWarning + ' ha warning [foo]' +
'\n';
Expand Down
58 changes: 55 additions & 3 deletions test/reporter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,16 @@ test('reporter with simple mock result containing non warning typed message', fu
plugin: 'foo',
text: 'foo warning',
},
{
type: 'error',
plugin: 'foo',
text: 'foo error',
},
];
var testReporter = reporter({
formatter: mockFormatter(tracker),
});
t.doesNotThrow(function() {
t.throws(function() {
testReporter.OnceExit(null, {
result: mockResultContainingNonWarningMessage,
});
Expand Down Expand Up @@ -158,7 +163,11 @@ test('reporter with simple mock result and function-filtered plugins', function(
formatter: mockFormatter(tracker),
filter: function(message) { return message.type === 'error'; },
});
testReporter.OnceExit(null, { result: cloneResult });
t.throws(function() {
testReporter.OnceExit(null, {
result: cloneResult,
});
});
t.deepEqual(
tracker.messages,
[
Expand Down Expand Up @@ -422,7 +431,11 @@ test('reporter with warnings that messages that each have nodes', function(t) {
var testReporter = reporter({
formatter: mockMultiSourceFormatter(tracker),
});
testReporter.OnceExit(null, { result: mockWarningNodeResult });
t.throws(function() {
testReporter.OnceExit(null, {
result: mockWarningNodeResult,
});
});
t.deepEqual(tracker, [
{
source: 'foo.css',
Expand Down Expand Up @@ -470,6 +483,45 @@ test('reporter with warnings that messages that each have nodes', function(t) {
},
],
},
{
source: '<input css 2>',
messages: [
{
type: 'error',
plugin: 'pat',
text: 'pat error',
node: {
source: {
input: {
id: '<input css 2>',
},
},
},
},
{
type: 'error',
plugin: 'hoo',
text: 'hoo error',
node: {
source: {
input: {
id: '<input css 2>',
},
},
},
},
],
},
{
source: '<input css 1>',
messages: [
{
type: 'error',
plugin: 'hah',
text: 'hah error',
},
],
},
]);
t.end();
});

1 comment on commit 7109853

@fredli74
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes it throw on any message. In my project, I use

    reporter({
      clearReportedMessages: true,
      throwError: true,
      filter: function() { return true; }
    }),

Because I want it to output all messages and throw on both errors and warnings. But return opts.throwError || errorCount > 0; makes it always throw.

Please sign in to comment.