refactor(api): consistent VALIDATE messages #22262

Problem:
Validation messages are not consistently formatted.
- Parameter names sometimes are NOT quoted.
- Descriptive names (non-parameters) sometimes ARE quoted.

Solution:
Always quote the `name` value passed to a VALIDATE macro _unless_ the
value has whitespace.
This commit is contained in:
Justin M. Keyes
2023-02-14 14:19:28 -05:00
committed by GitHub
parent e03ecb7d31
commit 556f8646c0
18 changed files with 215 additions and 139 deletions

View File

@@ -108,19 +108,19 @@ Array nvim_get_autocmds(Dict(get_autocmds) *opts, Error *err)
break;
case kObjectTypeString:
group = augroup_find(opts->group.data.string.data);
VALIDATE_S((group >= 0), "group", "", {
VALIDATE_S((group >= 0), "group", opts->group.data.string.data, {
goto cleanup;
});
break;
case kObjectTypeInteger:
group = (int)opts->group.data.integer;
char *name = augroup_name(group);
VALIDATE_S(augroup_exists(name), "group", "", {
VALIDATE_INT(augroup_exists(name), "group", opts->group.data.integer, {
goto cleanup;
});
break;
default:
VALIDATE_S(false, "group (must be string or integer)", "", {
VALIDATE_EXP(false, "group", "String or Integer", api_typename(opts->group.type), {
goto cleanup;
});
}
@@ -142,7 +142,7 @@ Array nvim_get_autocmds(Dict(get_autocmds) *opts, Error *err)
event_set[event_nr] = true;
})
} else {
VALIDATE_S(false, "event (must be String or Array)", "", {
VALIDATE_EXP(false, "event", "String or Array", NULL, {
goto cleanup;
});
}
@@ -160,11 +160,10 @@ Array nvim_get_autocmds(Dict(get_autocmds) *opts, Error *err)
pattern_filters[pattern_filter_count] = v.data.string.data;
pattern_filter_count += 1;
} else if (v.type == kObjectTypeArray) {
if (v.data.array.size > AUCMD_MAX_PATTERNS) {
api_set_error(err, kErrorTypeValidation, "Too many patterns (maximum of %d)",
AUCMD_MAX_PATTERNS);
VALIDATE((v.data.array.size <= AUCMD_MAX_PATTERNS),
"Too many patterns (maximum of %d)", AUCMD_MAX_PATTERNS, {
goto cleanup;
}
});
FOREACH_ITEM(v.data.array, item, {
VALIDATE_T("pattern", kObjectTypeString, item.type, {

View File

@@ -207,7 +207,7 @@ Boolean nvim_buf_attach(uint64_t channel_id, Buffer buffer, Boolean send_buffer,
}
}
VALIDATE_S(key_used, "key", k.data, {
VALIDATE_S(key_used, "'opts' key", k.data, {
goto error;
});
}
@@ -1074,7 +1074,7 @@ void nvim_buf_delete(Buffer buffer, Dictionary opts, Error *err)
} else if (strequal("unload", k.data)) {
unload = api_object_to_bool(v, "unload", false, err);
} else {
VALIDATE_S(false, "key", k.data, {
VALIDATE_S(false, "'opts' key", k.data, {
return;
});
}

View File

@@ -1017,7 +1017,7 @@ void create_user_command(String name, Object command, Dict(user_command) *opts,
VALIDATE_S(uc_validate_name(name.data), "command name", name.data, {
goto err;
});
VALIDATE_S(!mb_islower(name.data[0]), "command name (must begin with an uppercase letter)",
VALIDATE_S(!mb_islower(name.data[0]), "command name (must start with uppercase)",
name.data, {
goto err;
});
@@ -1163,7 +1163,7 @@ void create_user_command(String name, Object command, Dict(user_command) *opts,
goto err;
});
} else if (HAS_KEY(opts->complete)) {
VALIDATE(false, "%s", "Invalid complete: expected Function or String", {
VALIDATE_EXP(false, "complete", "Function or String", NULL, {
goto err;
});
}
@@ -1190,7 +1190,7 @@ void create_user_command(String name, Object command, Dict(user_command) *opts,
rep = command.data.string.data;
break;
default:
VALIDATE(false, "%s", "Invalid command: expected Function or String", {
VALIDATE_EXP(false, "command", "Function or String", NULL, {
goto err;
});
}

View File

@@ -238,7 +238,7 @@ ArrayOf(Integer) nvim_buf_get_extmark_by_id(Buffer buffer, Integer ns_id,
});
}
} else {
VALIDATE_S(false, "key", k.data, {
VALIDATE_S(false, "'opts' key", k.data, {
return rv;
});
}
@@ -329,7 +329,7 @@ Array nvim_buf_get_extmarks(Buffer buffer, Integer ns_id, Object start, Object e
});
}
} else {
VALIDATE_S(false, "key", k.data, {
VALIDATE_S(false, "'opts' key", k.data, {
return rv;
});
}
@@ -508,12 +508,13 @@ Integer nvim_buf_set_extmark(Buffer buffer, Integer ns_id, Integer line, Integer
});
uint32_t id = 0;
if (opts->id.type == kObjectTypeInteger && opts->id.data.integer > 0) {
id = (uint32_t)opts->id.data.integer;
} else if (HAS_KEY(opts->id)) {
VALIDATE_S(false, "id (must be positive integer)", "", {
if (HAS_KEY(opts->id)) {
VALIDATE_EXP((opts->id.type == kObjectTypeInteger && opts->id.data.integer > 0),
"id", "positive Integer", NULL, {
goto error;
});
id = (uint32_t)opts->id.data.integer;
}
int line2 = -1;
@@ -1088,10 +1089,10 @@ static bool extmark_get_index_from_obj(buf_T *buf, Integer ns_id, Object obj, in
// Check if it is a position
} else if (obj.type == kObjectTypeArray) {
Array pos = obj.data.array;
VALIDATE((pos.size == 2
&& pos.items[0].type == kObjectTypeInteger
&& pos.items[1].type == kObjectTypeInteger),
"%s", "Invalid position: expected 2 Integer items", {
VALIDATE_EXP((pos.size == 2
&& pos.items[0].type == kObjectTypeInteger
&& pos.items[1].type == kObjectTypeInteger),
"mark position", "2 Integer items", NULL, {
return false;
});
@@ -1101,7 +1102,7 @@ static bool extmark_get_index_from_obj(buf_T *buf, Integer ns_id, Object obj, in
*col = (colnr_T)(pos_col >= 0 ? pos_col : MAXCOL);
return true;
} else {
VALIDATE(false, "%s", "Invalid position: expected mark id Integer or 2-item Array", {
VALIDATE_EXP(false, "mark position", "mark id Integer or 2-item Array", NULL, {
return false;
});
}

View File

@@ -36,7 +36,7 @@ static int validate_option_value_args(Dict(option) *opts, int *scope, int *opt_t
} else if (!strcmp(opts->scope.data.string.data, "global")) {
*scope = OPT_GLOBAL;
} else {
VALIDATE(false, "%s", "Invalid scope: expected 'local' or 'global'", {
VALIDATE_EXP(false, "scope", "'local' or 'global'", NULL, {
return FAIL;
});
}
@@ -197,7 +197,7 @@ void nvim_set_option_value(String name, Object value, Dict(option) *opts, Error
scope |= OPT_CLEAR;
break;
default:
VALIDATE_EXP(false, "option type", "Integer, Boolean, or String", api_typename(value.type), {
VALIDATE_EXP(false, name.data, "Integer/Boolean/String", api_typename(value.type), {
return;
});
}

View File

@@ -9,6 +9,55 @@
# include "api/private/validate.c.generated.h"
#endif
/// Creates "Invalid …" message and sets it on `err`.
void api_err_invalid(Error *err, const char *name, const char *val_s, int64_t val_n, bool quote_val)
{
ErrorType errtype = kErrorTypeValidation;
// Treat `name` without whitespace as a parameter (surround in quotes).
// Treat `name` with whitespace as a description (no quotes).
char *has_space = strchr(name, ' ');
// No value.
if (val_s && val_s[0] == '\0') {
api_set_error(err, errtype, has_space ? "Invalid %s" : "Invalid '%s'", name);
return;
}
// Number value.
if (val_s == NULL) {
api_set_error(err, errtype, has_space ? "Invalid %s: %" PRId64 : "Invalid '%s': %" PRId64,
name, val_n);
return;
}
// String value.
if (has_space) {
api_set_error(err, errtype, quote_val ? "Invalid %s: '%s'" : "Invalid %s: %s", name, val_s);
} else {
api_set_error(err, errtype, quote_val ? "Invalid '%s': '%s'" : "Invalid '%s': %s", name, val_s);
}
}
/// Creates "Invalid …: expected …" message and sets it on `err`.
void api_err_exp(Error *err, const char *name, const char *expected, const char *actual)
{
ErrorType errtype = kErrorTypeValidation;
// Treat `name` without whitespace as a parameter (surround in quotes).
// Treat `name` with whitespace as a description (no quotes).
char *has_space = strchr(name, ' ');
if (!actual) {
api_set_error(err, errtype,
has_space ? "Invalid %s: expected %s" : "Invalid '%s': expected %s",
name, expected);
return;
}
api_set_error(err, errtype,
has_space ? "Invalid %s: expected %s, got %s" : "Invalid '%s': expected %s, got %s",
name, expected, actual);
}
bool check_string_array(Array arr, char *name, bool disallow_nl, Error *err)
{
snprintf(IObuff, sizeof(IObuff), "'%s' item", name);

View File

@@ -4,44 +4,6 @@
#include "nvim/api/private/defs.h"
#include "nvim/api/private/helpers.h"
#define VALIDATE_INT(cond, name, val_, code) \
do { \
if (!(cond)) { \
api_set_error(err, kErrorTypeValidation, "Invalid " name ": %" PRId64, val_); \
code; \
} \
} while (0)
#define VALIDATE_S(cond, name, val_, code) \
do { \
if (!(cond)) { \
if (strequal(val_, "")) { \
api_set_error(err, kErrorTypeValidation, "Invalid " name); \
} else { \
api_set_error(err, kErrorTypeValidation, "Invalid " name ": '%s'", val_); \
} \
code; \
} \
} while (0)
#define VALIDATE_EXP(cond, name, expected, actual, code) \
do { \
if (!(cond)) { \
api_set_error(err, kErrorTypeValidation, "Invalid %s: expected %s, got %s", \
name, expected, actual); \
code; \
} \
} while (0)
#define VALIDATE_T(name, expected_t, actual_t, code) \
do { \
if (expected_t != actual_t) { \
api_set_error(err, kErrorTypeValidation, "Invalid %s: expected %s, got %s", \
name, api_typename(expected_t), api_typename(actual_t)); \
code; \
} \
} while (0)
#define VALIDATE(cond, fmt_, fmt_arg1, code) \
do { \
if (!(cond)) { \
@@ -50,10 +12,42 @@
} \
} while (0)
#define VALIDATE_INT(cond, name, val_, code) \
do { \
if (!(cond)) { \
api_err_invalid(err, name, NULL, val_, false); \
code; \
} \
} while (0)
#define VALIDATE_S(cond, name, val_, code) \
do { \
if (!(cond)) { \
api_err_invalid(err, name, val_, 0, true); \
code; \
} \
} while (0)
#define VALIDATE_EXP(cond, name, expected, actual, code) \
do { \
if (!(cond)) { \
api_err_exp(err, name, expected, actual); \
code; \
} \
} while (0)
#define VALIDATE_T(name, expected_t, actual_t, code) \
do { \
if (expected_t != actual_t) { \
api_err_exp(err, name, api_typename(expected_t), api_typename(actual_t)); \
code; \
} \
} while (0)
#define VALIDATE_RANGE(cond, name, code) \
do { \
if (!(cond)) { \
api_set_error(err, kErrorTypeValidation, "Invalid '%s': out of range", name); \
api_err_invalid(err, name, "out of range", 0, false); \
code; \
} \
} while (0)

View File

@@ -654,12 +654,13 @@ Object nvim_get_var(String name, Error *err)
{
dictitem_T *di = tv_dict_find(&globvardict, name.data, (ptrdiff_t)name.size);
if (di == NULL) { // try to autoload script
VALIDATE_S((script_autoload(name.data, name.size, false) && !aborting()), "key", name.data, {
bool found = script_autoload(name.data, name.size, false) && !aborting();
VALIDATE(found, "Key not found: %s", name.data, {
return (Object)OBJECT_INIT;
});
di = tv_dict_find(&globvardict, name.data, (ptrdiff_t)name.size);
}
VALIDATE_S((di != NULL), "key (not found)", name.data, {
VALIDATE((di != NULL), "Key not found: %s", name.data, {
return (Object)OBJECT_INIT;
});
return vim_to_object(&di->di_tv);
@@ -986,7 +987,7 @@ Integer nvim_open_term(Buffer buffer, DictionaryOf(LuaRef) opts, Error *err)
v->data.luaref = LUA_NOREF;
break;
} else {
VALIDATE_S(false, "key", k.data, {});
VALIDATE_S(false, "'opts' key", k.data, {});
}
}
@@ -1632,18 +1633,18 @@ Array nvim_call_atomic(uint64_t channel_id, Array calls, Arena *arena, Error *er
size_t i; // also used for freeing the variables
for (i = 0; i < calls.size; i++) {
VALIDATE_T("calls item", kObjectTypeArray, calls.items[i].type, {
VALIDATE_T("'calls' item", kObjectTypeArray, calls.items[i].type, {
goto theend;
});
Array call = calls.items[i].data.array;
VALIDATE((call.size == 2), "%s", "calls item must be a 2-item Array", {
VALIDATE_EXP((call.size == 2), "'calls' item", "2-item Array", NULL, {
goto theend;
});
VALIDATE_T("name", kObjectTypeString, call.items[0].type, {
goto theend;
});
String name = call.items[0].data.string;
VALIDATE_T("args", kObjectTypeArray, call.items[1].type, {
VALIDATE_T("call args", kObjectTypeArray, call.items[1].type, {
goto theend;
});
Array args = call.items[1].data.array;
@@ -2108,10 +2109,10 @@ Dictionary nvim_eval_statusline(String str, Dict(eval_statusline) *opts, Error *
VALIDATE_T("fillchar", kObjectTypeString, opts->fillchar.type, {
return result;
});
VALIDATE((opts->fillchar.data.string.size != 0
&& ((size_t)utf_ptr2len(opts->fillchar.data.string.data)
== opts->fillchar.data.string.size)),
"%s", "Invalid fillchar: expected single character", {
VALIDATE_EXP((opts->fillchar.data.string.size != 0
&& ((size_t)utf_ptr2len(opts->fillchar.data.string.data)
== opts->fillchar.data.string.size)),
"fillchar", "single character", NULL, {
return result;
});
fillchar = utf_ptr2char(opts->fillchar.data.string.data);