Continue to refactor functions

Now that we have immutable props, we can remove a bunch of 'helper'
functions.
This commit is contained in:
ridiculousfish 2021-10-21 14:28:39 -07:00
parent 7d7b930b08
commit 4a6d622733
7 changed files with 79 additions and 119 deletions

View file

@ -135,7 +135,7 @@ static int parse_cmd_opts(functions_cmd_opts_t &opts, int *optind, //!OCLINT(hi
return STATUS_CMD_OK;
}
static int report_function_metadata(const wchar_t *funcname, bool verbose, io_streams_t &streams,
static int report_function_metadata(const wcstring &funcname, bool verbose, io_streams_t &streams,
parser_t &parser, bool metadata_as_comments) {
const wchar_t *path = L"n/a";
const wchar_t *autoloaded = L"n/a";
@ -143,18 +143,16 @@ static int report_function_metadata(const wchar_t *funcname, bool verbose, io_st
wcstring description = L"n/a";
int line_number = 0;
if (function_exists(funcname, parser)) {
auto props = function_get_props(funcname);
path = function_get_definition_file(funcname);
if (auto props = function_get_props_autoload(funcname, parser)) {
path = props->definition_file;
if (path) {
autoloaded = function_is_autoloaded(funcname) ? L"autoloaded" : L"not-autoloaded";
line_number = function_get_definition_lineno(funcname);
autoloaded = props->is_autoload ? L"autoloaded" : L"not-autoloaded";
line_number = props->definition_lineno();
} else {
path = L"stdin";
}
shadows_scope = props->shadow_scope ? L"scope-shadowing" : L"no-scope-shadowing";
function_get_desc(funcname, description);
description = escape_string(description, ESCAPE_NO_QUOTED);
description = escape_string(props->description, ESCAPE_NO_QUOTED);
}
if (metadata_as_comments) {
@ -347,16 +345,17 @@ maybe_t<int> builtin_functions(parser_t &parser, io_streams_t &streams, const wc
int res = STATUS_CMD_OK;
for (int i = optind; i < argc; i++) {
if (!function_exists(argv[i], parser)) {
wcstring funcname = argv[i];
auto func = function_get_props_autoload(argv[i], parser);
if (!func) {
res++;
} else {
if (!opts.query) {
if (i != optind) streams.out.append(L"\n");
const wchar_t *funcname = argv[i];
if (!opts.no_metadata) {
report_function_metadata(funcname, opts.verbose, streams, parser, true);
}
wcstring def = functions_def(funcname);
wcstring def = func->annotated_definition(funcname);
if (!streams.out_is_redirected && isatty(STDOUT_FILENO)) {
std::vector<highlight_spec_t> colors;

View file

@ -125,11 +125,13 @@ maybe_t<int> builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t
int found = 0;
const wchar_t *name = argv[idx];
// Functions
if (!opts.force_path && !opts.no_functions && function_exists(name, parser)) {
function_properties_ref_t func{};
if (!opts.force_path && !opts.no_functions &&
(func = function_get_props_autoload(name, parser))) {
++found;
res = true;
if (!opts.query && !opts.type) {
auto path = function_get_definition_file(name);
auto path = func->definition_file;
if (opts.path) {
if (path) {
streams.out.append(path);
@ -140,9 +142,9 @@ maybe_t<int> builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t
streams.out.append(_(L" with definition"));
streams.out.append(L"\n");
// Function path
wcstring def = functions_def(name);
wcstring def = func->annotated_definition(name);
if (path) {
int line_number = function_get_definition_lineno(name);
int line_number = func->definition_lineno();
wcstring comment;
if (std::wcscmp(path, L"-") != 0) {
append_format(comment, L"# Defined in %ls @ line %d\n", path,
@ -165,7 +167,7 @@ maybe_t<int> builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t
}
} else {
streams.out.append_format(_(L"%ls is a function"), name);
auto path = function_get_definition_file(name);
auto path = func->definition_file;
if (path) {
streams.out.append_format(_(L" (defined in %ls)"), path);
}

View file

@ -660,9 +660,10 @@ void completer_t::complete_cmd_desc(const wcstring &str) {
/// Returns a description for the specified function, or an empty string if none.
static wcstring complete_function_desc(const wcstring &fn) {
wcstring result;
function_get_desc(fn, result);
return result;
if (auto props = function_get_props(fn)) {
return props->description;
}
return wcstring{};
}
/// Complete the specified command name. Search for executables in the path, executables defined
@ -854,7 +855,7 @@ static void complete_load(const wcstring &name) {
// We have to load this as a function, since it may define a --wraps or signature.
// See issue #2466.
auto &parser = parser_t::principal_parser();
function_load(name, parser);
function_get_props_autoload(name, parser);
// It's important to NOT hold the lock around completion loading.
// We need to take the lock to decide what to load, drop it to perform the load, then reacquire

View file

@ -163,20 +163,17 @@ function_properties_ref_t function_get_props(const wcstring &name) {
return function_set.acquire()->get_props(name);
}
int function_exists(const wcstring &cmd, parser_t &parser) {
function_properties_ref_t function_get_props_autoload(const wcstring &name, parser_t &parser) {
ASSERT_IS_MAIN_THREAD();
if (!valid_func_name(cmd)) return false;
if (parser_keywords_is_reserved(cmd)) return 0;
try_autoload(cmd, parser);
auto funcset = function_set.acquire();
return funcset->funcs.find(cmd) != funcset->funcs.end();
if (parser_keywords_is_reserved(name)) return nullptr;
try_autoload(name, parser);
return function_get_props(name);
}
void function_load(const wcstring &cmd, parser_t &parser) {
bool function_exists(const wcstring &cmd, parser_t &parser) {
ASSERT_IS_MAIN_THREAD();
if (!parser_keywords_is_reserved(cmd)) {
try_autoload(cmd, parser);
}
if (!valid_func_name(cmd)) return false;
return function_get_props_autoload(cmd, parser) != nullptr;
}
bool function_exists_no_autoload(const wcstring &cmd) {
@ -203,29 +200,19 @@ void function_remove(const wcstring &name) {
funcset->autoload_tombstones.insert(name);
}
bool function_get_definition(const wcstring &name, wcstring &out_definition) {
auto props = function_get_props(name);
if (!props) return false;
// \return the body of a function (everything after the header, up to but not including the 'end').
static wcstring get_function_body_source(const function_properties_t &props) {
// We want to preserve comments that the AST attaches to the header (#5285).
// Take everything from the end of the header to the 'end' keyword.
auto header_src = props->func_node->header->try_source_range();
auto end_kw_src = props->func_node->end.try_source_range();
auto header_src = props.func_node->header->try_source_range();
auto end_kw_src = props.func_node->end.try_source_range();
if (header_src && end_kw_src) {
uint32_t body_start = header_src->start + header_src->length;
uint32_t body_end = end_kw_src->start;
assert(body_start <= body_end && "end keyword should come after header");
out_definition = wcstring(props->parsed_source->src, body_start, body_end - body_start);
return wcstring(props.parsed_source->src, body_start, body_end - body_start);
}
return true;
}
bool function_get_desc(const wcstring &name, wcstring &out_desc) {
if (auto props = function_get_props(name)) {
out_desc = _(props->description.c_str());
return true;
}
return false;
return wcstring{};
}
void function_set_desc(const wcstring &name, const wcstring &desc, parser_t &parser) {
@ -278,34 +265,6 @@ wcstring_list_t function_get_names(int get_hidden) {
return wcstring_list_t(names.begin(), names.end());
}
const wchar_t *function_get_definition_file(const wcstring &name) {
if (auto func = function_get_props(name)) {
return func->definition_file;
}
return nullptr;
}
bool function_is_autoloaded(const wcstring &name) {
if (auto func = function_get_props(name)) {
return func->is_autoload;
}
return false;
}
int function_get_definition_lineno(const wcstring &name) {
const auto props = function_set.acquire()->get_props(name);
if (!props) return -1;
// return one plus the number of newlines at offsets less than the start of our function's
// statement (which includes the header).
// TODO: merge with line_offset_of_character_at_offset?
auto source_range = props->func_node->try_source_range();
assert(source_range && "Function has no source range");
uint32_t func_start = source_range->start;
const wcstring &source = props->parsed_source->src;
assert(func_start <= source.size() && "function start out of bounds");
return 1 + std::count(source.begin(), source.begin() + func_start, L'\n');
}
void function_invalidate_path() {
// Remove all autoloaded functions and update the autoload path.
// Note we don't want to risk removal during iteration; we expect this to be called
@ -323,13 +282,10 @@ void function_invalidate_path() {
funcset->autoloader.clear();
}
/// Return a definition of the specified function. Used by the functions builtin.
wcstring functions_def(const wcstring &name) {
assert(!name.empty() && "Empty name");
wcstring function_properties_t::annotated_definition(const wcstring &name) const {
wcstring out;
wcstring desc, def;
function_get_desc(name, desc);
function_get_definition(name, def);
wcstring desc = this->localized_description();
wcstring def = get_function_body_source(*this);
std::vector<std::shared_ptr<event_handler_t>> ev = event_get_function_handlers(name);
out.append(L"function ");
@ -353,9 +309,7 @@ wcstring functions_def(const wcstring &name) {
out.append(esc_desc);
}
auto props = function_get_props(name);
assert(props && "Should have function properties");
if (!props->shadow_scope) {
if (!this->shadow_scope) {
out.append(L" --no-scope-shadowing");
}
@ -393,7 +347,7 @@ wcstring functions_def(const wcstring &name) {
}
}
const wcstring_list_t &named = props->named_arguments;
const wcstring_list_t &named = this->named_arguments;
if (!named.empty()) {
append_format(out, L" --argument");
for (const auto &name : named) {
@ -408,7 +362,7 @@ wcstring functions_def(const wcstring &name) {
}
// Output any inherited variables as `set -l` lines.
for (const auto &kv : props->inherit_vars) {
for (const auto &kv : this->inherit_vars) {
// We don't know what indentation style the function uses,
// so we do what fish_indent would.
append_format(out, L"\n set -l %ls", kv.first.c_str());
@ -428,3 +382,20 @@ wcstring functions_def(const wcstring &name) {
out.append(L"end\n");
return out;
}
const wchar_t *function_properties_t::localized_description() const {
if (description.empty()) return L"";
return _(description.c_str());
}
int function_properties_t::definition_lineno() const {
// return one plus the number of newlines at offsets less than the start of our function's
// statement (which includes the header).
// TODO: merge with line_offset_of_character_at_offset?
auto source_range = func_node->try_source_range();
assert(source_range && "Function has no source range");
uint32_t func_start = source_range->start;
const wcstring &source = parsed_source->src;
assert(func_start <= source.size() && "function start out of bounds");
return 1 + std::count(source.begin(), source.begin() + func_start, L'\n');
}

View file

@ -47,6 +47,17 @@ struct function_properties_t {
/// The file from which the function was created (intern'd string), or nullptr if not from a
/// file.
const wchar_t *definition_file{};
/// \return the description, localized via _.
const wchar_t *localized_description() const;
/// \return the line number where the definition of the specified function started.
int definition_lineno() const;
/// \return a definition of the function, annotated with properties like event handlers and wrap
/// targets. This is to support the 'functions' builtin.
/// Note callers must provide the function name, since the function does not know its own name.
wcstring annotated_definition(const wcstring &name) const;
};
using function_properties_ref_t = std::shared_ptr<const function_properties_t>;
@ -60,27 +71,19 @@ void function_remove(const wcstring &name);
/// \return the properties for a function, or nullptr if none. This does not trigger autoloading.
function_properties_ref_t function_get_props(const wcstring &name);
/// Returns by reference the definition of the function with the name \c name. Returns true if
/// successful, false if no function with the given name exists.
/// This does not trigger autoloading.
bool function_get_definition(const wcstring &name, wcstring &out_definition);
/// Returns by reference the description of the function with the name \c name. Returns true if the
/// function exists and has a nonempty description, false if it does not.
/// This does not trigger autoloading.
bool function_get_desc(const wcstring &name, wcstring &out_desc);
/// \return the properties for a function, or nullptr if none, perhaps triggering autoloading.
function_properties_ref_t function_get_props_autoload(const wcstring &name, parser_t &parser);
/// Sets the description of the function with the name \c name.
/// This triggers autoloading.
void function_set_desc(const wcstring &name, const wcstring &desc, parser_t &parser);
/// Returns true if the function with the name name exists.
/// Returns true if the function named \p cmd exists.
/// This may autoload.
int function_exists(const wcstring &cmd, parser_t &parser);
bool function_exists(const wcstring &cmd, parser_t &parser);
/// Attempts to load a function if not yet loaded. This is used by the completion machinery.
void function_load(const wcstring &cmd, parser_t &parser);
/// Returns true if the function with the name name exists, without triggering autoload.
/// Returns true if the function \p cmd either is loaded, or exists on disk in an autoload
/// directory.
bool function_exists_no_autoload(const wcstring &cmd);
/// Returns all function names.
@ -88,22 +91,6 @@ bool function_exists_no_autoload(const wcstring &cmd);
/// \param get_hidden whether to include hidden functions, i.e. ones starting with an underscore.
wcstring_list_t function_get_names(int get_hidden);
/// Returns true if the function was autoloaded.
bool function_is_autoloaded(const wcstring &name);
/// Returns tha absolute path of the file where the specified function was defined. Returns 0 if the
/// file was defined on the commandline.
///
/// This function does not autoload functions, it will only work on functions that have already been
/// defined.
///
/// This returns an intern'd string.
const wchar_t *function_get_definition_file(const wcstring &name);
/// Returns the linenumber where the definition of the specified function started.
/// This does not trigger autoloading.
int function_get_definition_lineno(const wcstring &name);
/// Creates a new function using the same definition as the specified function. Returns true if copy
/// is successful.
bool function_copy(const wcstring &name, const wcstring &new_name);
@ -111,5 +98,4 @@ bool function_copy(const wcstring &name, const wcstring &new_name);
/// Observes that fish_function_path has changed.
void function_invalidate_path();
wcstring functions_def(const wcstring &name);
#endif

View file

@ -78,7 +78,7 @@ void parse_util_token_extent(const wchar_t *buff, size_t cursor_pos, const wchar
const wchar_t **tok_end, const wchar_t **prev_begin,
const wchar_t **prev_end);
/// Get the linenumber at the specified character offset.
/// Get the line number at the specified character offset.
int parse_util_lineno(const wcstring &str, size_t offset);
/// Calculate the line number of the specified cursor position.

View file

@ -451,7 +451,8 @@ int parser_t::get_lineno() const {
const wchar_t *parser_t::current_filename() const {
for (const auto &b : block_list) {
if (b.is_function_call()) {
return function_get_definition_file(b.function_name);
auto props = function_get_props(b.function_name);
return props ? props->definition_file : nullptr;
} else if (b.type() == block_type_t::source) {
return b.sourced_file;
}