spacecruft
/
Farmbot-Web-App
1
0
Fork 0
Code Issues Pull Requests Releases Activity

Friendlier messages for CS Arg pairing errors.

pull/1097/head
Rick Carlino 2019-01-20 12:44:41 -06:00
parent aebcfaec5e
commit e92b49d57d
4 changed files with 73 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
app/lib/celery_script/checker.rb
View File

@ -5,13 +5,24 @@ module CeleryScript
class Checker
MISSING_ARG = "Expected node '%s' to have a '%s', but got: %s."
EXTRA_ARGS = "'%s' has unexpected arguments: %s. Allowed arguments: %s"
BAD_LEAF = "Expected leaf '%s' within '%s' to be one of: %s but got %s"
BAD_LEAF = "Expected leaf '%{kind}' within '%{parent_kind}'"\
" to be one of: %{allowed} but got %{actual}"
MALFORMED = "Expected '%s' to be a node or leaf, but it was neither"
BAD_BODY = "Body of '%s' node contains '%s' node. "\
"Expected one of: %s"
UNBOUND_VAR = "Unbound variable: %s"
T_MISMATCH = "Type mismatch. %s must be one of: %s. Got: %s"
# Certain CeleryScript pairing errors are more than just a syntax error.
# For instance, A `nothing` node in a `variable_declaration` is often an
# indication that the user did not fill out a value for a variable. In these
# rare cases, we muct provide information beyond what is found in the
# BAD_LEAF template.
FRIENDLY_ERRORS = {
nothing: {
variable_declaration: "You must provide a value for all parameters"
}
}.with_indifferent_access
attr_reader :tree, :corpus, :device
# Device is required for security / permission checks.
@ -43,11 +54,8 @@ module CeleryScript
def check_leaf(node)
allowed = corpus.values(node)
actual = node.value.class
ok_leaf = allowed.include?(actual)
raise TypeCheckError, (BAD_LEAF % [ node.kind,
node.parent.kind,
allowed.inspect,
actual.inspect]) unless ok_leaf
maybe_bad_leaf(node.kind, node.parent.kind, allowed, actual)
end
private
@ -152,21 +160,27 @@ module CeleryScript
# SEE_MY_NOTE =============================^ RC 4 Oct 18
end
end
ok = allowed.include?(actual)
raise TypeCheckError, (BAD_LEAF % [ value.kind,
value.parent.kind,
allowed.inspect,
actual.inspect ]) unless ok
maybe_bad_leaf(value.kind, value.parent.kind, allowed, actual)
end
def maybe_bad_leaf(kind, parent_kind, allowed, actual)
unless allowed.include?(actual)
message = (FRIENDLY_ERRORS.dig(kind, parent_kind) || BAD_LEAF) % {
kind: kind,
parent_kind: parent_kind,
allowed: allowed,
actual: actual
}
raise TypeCheckError, message
end
end
def validate_leaf_pairing(key, value)
actual = value.value.class
allowed = corpus.fetchArg(key).allowed_values
ok = allowed.include?(actual)
raise TypeCheckError, (BAD_LEAF % [ value.kind,
value.parent.kind,
allowed.inspect,
actual.inspect ]) unless ok
maybe_bad_leaf(value.kind, value.parent.kind, allowed, actual)
end
def bad_body_kind(prnt, child, i, ok)

9
app/models/celery_script_settings_bag.rb
View File

@ -78,8 +78,9 @@ module CeleryScriptSettingsBag
ONLY_ONE_COORD = "Move Absolute does not accept a group of locations"\
" as input. Please change your selection to a "\
"single location."
ALLOWED_EVERY_POINT_TYPE = %w(Tool GenericPointer Plant ToolSlot)
BAD_EVERY_POINT_TYPE = '"%s" is not a type of group. Allowed values: %s'
SCOPE_DECLARATIONS = [:variable_declaration, :parameter_declaration]
ALLOWED_EVERY_POINT_TYPE = %w(Tool GenericPointer Plant ToolSlot)
BAD_EVERY_POINT_TYPE = '"%s" is not a type of group. Allowed values: %s'
Corpus = CeleryScript::Corpus.new
.arg(:_else, [:execute, :nothing])
@ -249,7 +250,7 @@ module CeleryScriptSettingsBag
.node(:install_farmware, [:url])
.node(:update_farmware, [:package])
.node(:remove_farmware, [:package])
.node(:scope_declaration, [], [:variable_declaration, :parameter_declaration])
.node(:scope_declaration, [], SCOPE_DECLARATIONS)
.node(:identifier, [:label])
.node(:variable_declaration, [:label, :data_value], [])
.node(:parameter_declaration, [:label, :data_type], [])
@ -258,7 +259,7 @@ module CeleryScriptSettingsBag
.node(:dump_info, [], [])
.node(:install_first_party_farmware, [])
.node(:internal_farm_event, [], [:variable_declaration])
.node(:internal_regimen, [], [:variable_declaration, :parameter_declaration])
.node(:internal_regimen, [], SCOPE_DECLARATIONS)
.node(:internal_entry_point, [], [])
.node(:every_point, [:every_point_type], [])
.node(:resource_update, RESOURCE_UPDATE_ARGS) do |x|

30
spec/controllers/api/sequences/sequences_create_spec.rb
View File

@ -251,7 +251,6 @@ describe Api::SequencesController do
end
it 'prevents type errors from bad identifier / binding combos' do
$lol = true
sign_in user
input = { name: "type mismatch",
args: {
@ -283,5 +282,34 @@ describe Api::SequencesController do
expect(response.status).to eq(422)
expect(json[:body]).to include("not a valid data_type")
end
it 'provides human readable errors for "nothing" mismatches' do
sign_in user
input = { name: "type mismatch",
args: {
locals: {
kind: "scope_declaration",
args: { },
body: [
{
kind: "variable_declaration",
args: {
label: "x",
data_value: {
kind: "nothing",
args: {}
}
}
}
]
}
},
body: [ ]
}
post :create, body: input.to_json, params: {format: :json}
expect(response.status).to eq(422)
expect(json[:body]).to include("must provide a value for all parameters")
end
end
end

11
spec/lib/celery_script/checker_spec.rb
View File

@ -264,7 +264,13 @@ describe CeleryScript::Checker do
body: [
{
kind: "variable_declaration",
args: { label: "parent", data_value: { kind: "nothing", args: { } } }
args: {
label: "parent",
data_value: {
kind: "nothing",
args: { }
}
}
}
]
}
@ -283,6 +289,7 @@ describe CeleryScript::Checker do
tree = CeleryScript::AstNode.new(ast)
chk = CeleryScript::Checker.new(tree, corpus, device)
expect(chk.valid?).to be false
expect(chk.error.message).to include('but got "nothing"')
message = "must provide a value for all parameters"
expect(chk.error.message).to include(message)
end
end