Skip to content

Commit

Permalink
Rhys/error messages (#140)
Browse files Browse the repository at this point in the history 
* update makefile

* improved error handling
  • Loading branch information
@wheresrhys
wheresrhys authored Apr 20, 2017
1 parent 5a888bf commit 37a26de
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 20 deletions.
52 changes: 37 additions & 15 deletions lib/parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@ function parseParams (params, func) {
// Convert keen query string into recognised tokens
function tokenise (inputString) {

const errorContext = (token, current) => {
const startContext = Math.max(0, current - (15 + token.length));
const startToken = current - token.length;
const endToken = current;
const endContext = current + 15;
return [
inputString.substr(startContext, startToken - startContext),
token,
inputString.substr(endToken, endContext - endToken)
]
}

let current = 0;
let tokens = [];

Expand All @@ -50,7 +62,8 @@ function tokenise (inputString) {
activeOperator += 'Open';
tokens.push({
type: 'paren',
value: '('
value: '(',
context: errorContext('(', current + 1)
});

current++;
Expand All @@ -62,7 +75,8 @@ function tokenise (inputString) {
if (char === ')') {
tokens.push({
type: 'paren',
value: ')'
value: ')',
context: errorContext(')', current + 1)
});
current++;
activeOperator = null;
Expand All @@ -73,7 +87,8 @@ function tokenise (inputString) {
if (char === ',') {
tokens.push({
type: 'comma',
value: ','
value: ',',
context: errorContext(',', current + 1)
});
current++;
continue;
Expand All @@ -96,7 +111,8 @@ function tokenise (inputString) {
}
tokens.push({
type: 'aggregator',
value: aggName
value: aggName,
context: errorContext(aggName, current)
});
activeOperator = 'aggregator';
continue;
Expand All @@ -112,7 +128,8 @@ function tokenise (inputString) {
}
tokens.push({
type: 'function',
value: funcName
value: funcName,
context: errorContext(funcName, current)
});
activeOperator = 'function';
continue;
Expand All @@ -131,14 +148,16 @@ function tokenise (inputString) {
}
tokens.push({
type: 'collection',
value: collectionName
value: collectionName,
context: errorContext(collectionName, current)
});
continue;
}

// otherwise we only allow any other characters to appear if they are within a function body
if (activeOperator !== 'functionOpen') {
throw new TypeError('Invalid character: ' + char);
const context = errorContext(char, current + 1);
throw new TypeError('Invalid character: ' + char + ' in "' + context.join('') + '"');
}

let params = '';
Expand All @@ -158,11 +177,11 @@ function tokenise (inputString) {
}
tokens.push({
type: 'params',
value: params
value: params,
context: errorContext(params, current)
});
continue;
}

// return the tokenised string
return tokens;
}
Expand Down Expand Up @@ -229,9 +248,11 @@ function constructAST (tokens) {
// For functions we extract the name and parameters
if (token.type === 'function') {
const func = token.value;
const funcToken = token;
token = tokens[++current];
if (token.type !== 'paren' || token.value !== '(') {
throw new Error('functions must be followed by opening parenthesis');

if (!token || token.type !== 'paren' || token.value !== '(') {
throw new Error('functions must be followed by opening parenthesis: "' + funcToken.context.join('') + '"');
}
token = tokens[++current];
// Handling the case where the function has no parameters
Expand All @@ -247,10 +268,11 @@ function constructAST (tokens) {
// and the case where parameters exist, which we pass into the parmeter parser
// to convert into an array or keen-compatible filter config object
params = parseParams(token.value, func);
}
token = tokens[++current];
if (token.type !== 'paren' && token.value !== ')') {
throw new Error('functions have closing parenthesis');
const paramsToken = token;
token = tokens[++current];
if (!token || token.type !== 'paren' && token.value !== ')') {
throw new Error('functions must have closing parenthesis: "' + paramsToken.context.join('') + '"');
}
}
current++;
return {
Expand Down
14 changes: 9 additions & 5 deletions n.Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ NPM_INSTALL = npm prune --production=false && npm install
BOWER_INSTALL = bower prune && bower install --config.registry.search=http://registry.origami.ft.com --config.registry.search=https://bower.herokuapp.com
JSON_GET_VALUE = grep $1 | head -n 1 | sed 's/[," ]//g' | cut -d : -f 2
IS_GIT_IGNORED = grep -q $(if $1, $1, $@) .gitignore
VERSION = v15
VERSION = v23
APP_NAME = $(shell cat package.json 2>/dev/null | $(call JSON_GET_VALUE,name))
DONE = echo ✓ $@ done
CONFIG_VARS = curl -fsL https://ft-next-config-vars.herokuapp.com/$1/$(call APP_NAME)$(if $2,.$2,) -H "Authorization: `heroku config:get APIKEY --app ft-next-config-vars`"

IS_USER_FACING = `find . -type d \( -path ./bower_components -o -path ./node_modules \) -prune -o -name '*.html' -print`
MAKEFILE_HAS_A11Y = `grep -rli "make a11y" Makefile`

#
# META TASKS
Expand Down Expand Up @@ -95,7 +96,7 @@ ifneq ($(CIRCLE_BUILD_NUM),)
ifneq ($(shell grep -s -Fim 1 n-ui bower.json),)
# versions in package.json and bower.json are not equal
ifneq ($(shell awk '$$1 == "\"version\":" {print $$2}' bower_components/n-ui/.bower.json | sed s/,//),$(shell awk '$$1 == "\"version\":" {print $$2}' node_modules/@financial-times/n-ui/package.json | sed s/,//))
$(error 'Projects using n-ui must maintain parity between versions. Rebuild without cache and update your bower.json and package.json if necessary')
$(error 'Projects using n-ui must maintain parity between versions. Rebuild without cache and update your bower.json and package.json if necessary. If this error persists make sure that the n-ui build succeeded in publishing a new version to NPM and that both NPM and Bower registries have the latest version.')
endif
endif
endif
Expand Down Expand Up @@ -159,7 +160,7 @@ heroku-cli:
# VERIFY SUB-TASKS

_verify_eslint:
@if [ -e .eslintrc.js ]; then $(call GLOB,'*.js') | xargs eslint --fix --ignore-pattern '!' && $(DONE); fi
@if [ -e .eslintrc.js ]; then $(call GLOB,'*.js') | xargs eslint --ignore-pattern '!' && $(DONE); fi

_verify_lintspaces:
@if [ -e .editorconfig ] && [ -e package.json ]; then $(call GLOB) | xargs lintspaces -e .editorconfig -i js-comments -i html-comments && $(DONE); fi
Expand All @@ -169,8 +170,11 @@ _verify_scss_lint:
@if [ -e .scss-lint.yml ]; then { scss-lint -c ./.scss-lint.yml `$(call GLOB,'*.scss')`; if [ $$? -ne 0 -a $$? -ne 1 ]; then exit 1; fi; $(DONE); } fi

VERIFY_MSG_NO_DEMO = "Error: Components with templates must have a demo app, so that pa11y can test against it. This component doesn’t seem to have one. Add a demo app to continue peacefully. See n-image for an example."

VERIFY_MSG_NO_PA11Y = "\n**** Error ****\nIt looks like your code is user-facing; your Makefile should include make a11y\nIf you need to disable a11y, use export IGNORE_A11Y = true in your Makefile\n********\n\n"
#check if project has HTML and missing make a11y command
#check if project has demo app if there's a make a11y command
_verify_pa11y_testable:
@if [ "$(IS_USER_FACING)" ] && [ -z $(MAKEFILE_HAS_A11Y) ] && [ ! ${IGNORE_A11Y} ]; then (printf $(VERIFY_MSG_NO_PA11Y) && exit 1); fi
@if [ ! -d server ] && [ -d templates ] && [ ! -f demos/app.js ]; then (echo $(VERIFY_MSG_NO_DEMO) && exit 1); fi
@$(DONE)

Expand Down

0 comments on commit 37a26de

Please sign in to comment.