This package will scan verb code for common issues and warn you about them. You can edit/add/remove some checks if you don't want them, or update them to better fit your coding standards.
For ToastStunt, the preferred installation method is the MOO Package Manager. Its package repository may contain a newer Code Scanner release than this preserved copy.
The Code Scanner covers some practices used for MOO development on Sindome. Pull requests with portable checks that would be useful to the wider community are welcome.
This code was tested on stock LambdaMOO 1.8.1 with LambdaCore and on ToastStunt with a modified LambdaCore database. Related databases may require adaptation.
- Comment at the top of the verb for non-player facing verbs (good practice!)
- Nesting of for/if/while > MAX_NESTING (a variable you can define in your code).
- Argument scatter in player facing verbs
- Object Numbers in code (you should corify things for clarity (and later your own sanity))
- tostr usage inside a :tell (this is already tostr'd on #1)
- .location assignment instead of comparison (so you don't loop over a bunch of objects and move them instead of checking their location)
- assignment in if statements (doing assignment in if statements is valid in moo, and useful, but it's good to be warned of in general so you don't assign instead of compare by accident)
- forking (forks are useful but writing a $scheduler that lets you $schedule things to be run at specific times is better)
- verb length > MAX_LENGTH (a variable you can change to suit your needs)
Create an object. This example uses #79 as the parent, but the package does not rely
on parent behavior, so choose an appropriate parent for your database.
@create #78 named Code Scanner
Now you'll want to corify the reference to the Code Scanner, replace #97 with your new object number.
@corify #97 as code_scanner
This allows you to reference the object as $code_scanner.
Copy the code below into a text editor and change the object number from #97 to the
object number of your newly created Code Scanner.
;#97.description = {"MOO Code Scanner 1.1 by Brendan Butts <slither@sindome.org>", "", "Github: https://github.com/SevenEcks/lambda-moo-programming", "", "Usage: $code_scanner:scan_for_issues(OBJ, verbname)", "Usage: $code_scanner:display_issues($code_scanner:scan_for_issues(OBJ, verbname))", "", "If you integrate this with your @Program verb I recommend you make a copy of it first and test on that just in case! But you can always use .program if you mess up!"}
@create #12654 named Code Scanner Utils:Code,Scanner,Utils,Code Scanner Utils
;;#97.("aliases") = {"Code", "Scanner", "Utils", "Code Scanner Utils"}
;;#97.("description") = "This is a placeholder parent for all the $..._utils packages, to more easily find them and manipulate them. At present this object defines no useful verbs or properties. (Filfre.)"
;;#97.("object_size") = {19212, 1612285023}
;;#97.("instance_id") = "#12654-1541212667.02364"
@verb #97:"scan_for_issues" this none this
@program #97:scan_for_issues
{object, verbname, ?options = {}, ?code = {}} = args;
if (!code)
code = verb_code(object, verbname);
endif
verb_args = verb_args(object, verbname);
"max length before we start warning it's too long";
MAX_LENGTH_WARNING = 40;
"max nesting before we start warning";
MAX_NESTING_WARNING = 2;
"what do the args of an internal variable look like?";
internal_args = {"this", "none", "this"};
warnings = {};
max_nest = 0;
open_ifs = 0;
open_fors = 0;
open_whiles = 0;
forks = 0;
"first real line of code, not a comment";
first_real_line = 0;
"count of what line we are on";
count = 0;
"is this an internal (tnt) verb?";
internal = (internal_args == verb_args) ? 1 | 0;
for line in (code)
count = count + 1;
"check for opening comments";
if (((count == 1) && internal) && (!$code_scanner:match_comment(line)))
warnings = {@warnings, {"You did not include a comment on the first line describing the use and args of your verb.", count}};
endif
"check if we have found the first real line of code or if it's just a comment";
if ((!first_real_line) && (!$code_scanner:match_comment(line)))
first_real_line = count;
endif
"check for an argument scatter in a player facing verb";
if ((!internal) && $code_scanner:match_arg_scatter(line))
warnings = {@warnings, {"You have used an argument scatter in a verb that is not 'this none this'. This is not how it should work.", count}};
endif
"find nesting";
if ($code_scanner:match_if(line))
open_ifs = open_ifs + 1;
elseif ($code_scanner:match_for(line))
open_fors = open_fors + 1;
elseif ($code_scanner:match_while(line))
open_whiles = open_whiles + 1;
elseif ($code_scanner:match_endif(line))
open_ifs = open_ifs - 1;
elseif ($code_scanner:match_endfor(line))
open_fors = open_fors - 1;
elseif ($code_scanner:match_endwhile(line))
open_whiles = open_whiles - 1;
elseif ($code_scanner:match_fork(line))
forks = forks + 1;
elseif ($code_scanner:match_object(line))
warnings = {@warnings, {"You may have an object number in your code. This should be avoided. Use corified references ($) instead.", count}};
endif
"check for tostr usage in :tell verbs";
if ($code_scanner:match_tell_tostr(line))
warnings = {@warnings, {"tostr usage found inside a :tell call. This is undeeded as :tell will call tostr on all it's args.", count}};
endif
if ($code_scanner:match_location_assignment(line))
warnings = {@warnings, {"IMPORTANT!!!!!!!!!!!!!!!! You may have included a .location assignment INSTEAD of a comparison (= instead of ==)!", count}};
endif
if ($code_scanner:match_if_assignment(line))
warnings = {@warnings, {"You are doing an assignment '=' operation in an if statement, please confirm you didn't mean to do an equality check '=='.", count}};
endif
if ($code_scanner:match_recycler_valid(line))
warnings = {@warnings, {"You are doing an if($recycler:valid()) operation without a ! in front of it, are you SURE that you don't need the bang (!)?", count}};
endif
if ((current_nest = (open_fors + open_ifs) + open_whiles) > max_nest)
max_nest = current_nest;
endif
endfor
if (forks)
warnings = {@warnings, {"There is a fork() in this code. Please do not do this unless you know what you are doing. Consider using the $scheduler (or something with a heartbeat) instead.", 0}};
endif
if (max_nest > MAX_NESTING_WARNING)
warnings = {@warnings, {tostr("Max nesting of if/for/while's is ", max_nest, ". Try refactoring or extracting pieces to a new verb to get your max nesting to 2 or below."), 0}};
endif
length_of_verb = length(code);
if (length_of_verb > MAX_LENGTH_WARNING)
warnings = {@warnings, {tostr("This verb is ", length_of_verb, " lines long. Consider refactoring or extracting to a new verb to get your max nesting to ", MAX_LENGTH_WARNING, " or below."), 0}};
endif
return warnings;
.
@verb #97:"display_issues" this none this
@program #97:display_issues
":display_issues(LIST warnings) => none";
"takes the output of :scan_for_issues and displays it";
{warnings} = args;
for warning_set in (warnings)
{warning, line_number} = warning_set;
if (line_number)
player:tell(tostr("Warning on line ", line_number), ": ", warning);
else
player:tell("Warning", ": ", warning);
endif
endfor
"VMS NOTE display messages #22664 07/17/18 1:16";
"VMS VERSION 1.01";
"Last modified by Fengshui (#22664) on Tue Jul 17 13:16:35 2018 PDT";
.
@verb #97:"match_if" this none this
@program #97:match_if
":match_if(STR line) => bool";
{line} = args;
return match(line, "^[ ]*if ");
.
@verb #97:"match_for" this none this
@program #97:match_for
":match_for(STR line) => bool";
{line} = args;
return match(line, "^[ ]*for ");
.
@verb #97:"match_while" this none this
@program #97:match_while
":match_while(STR line) => bool";
{line} = args;
return match(line, "^[ ]*while ");
.
@verb #97:"match_endif" this none this
@program #97:match_endif
":match_endif(STR line) => bool";
{line} = args;
return match(line, "^[ ]*endif$");
.
@verb #97:"match_endfor" this none this
@program #97:match_endfor
":match_if(STR line) => bool";
{line} = args;
return match(line, "^[ ]*endfor$");
.
@verb #97:"match_endwhile" this none this
@program #97:match_endwhile
":match_endwhile(STR line) => bool";
{line} = args;
return match(line, "^[ ]*endwhile");
.
@verb #97:"match_fork" this none this
@program #97:match_fork
":match_fork(STR line) => bool";
{line} = args;
return match(line, "^[ ]*fork");
.
@verb #97:"match_object" this none this
@program #97:match_object
":match_object(STR line) => bool";
{line} = args;
return match(line, "#[0-9]+");
.
@verb #97:"match_tell_tostr" this none this
@program #97:match_tell_tostr
":match_tell_tostr(STR line) => bool";
{line} = args;
return match(line, ".*:tell(.*tostr(");
.
@verb #97:"match_comment" this none this
@program #97:match_comment
":match_comment(STR line) => bool";
{line} = args;
return match(line, "^[ ]*\"");
.
@verb #97:"match_arg_scatter" this none this
@program #97:match_arg_scatter
":match_arg_scatter(STR line) => bool";
{line} = args;
return match(line, "{.+} = args;");
.
@verb #97:"match_location_assignment" this none this
@program #97:match_location_assignment
":match_if(STR line) => bool";
{line} = args;
return match(line, "[ ]*if (.+.location = .+)");
.
@verb #97:"match_if_assignment" this none this
@program #97:match_if_assignment
":match_if_assignment(STR line) => bool";
"looks for assignment operators in if statements";
{line} = args;
return match(line, "^[ ]*if (.+ = .+)");
.
@verb #97:"match_corified_verb_anywhere" this none this
@program #97:match_corified_verb_anywhere
":match_corified(STR line) => bool";
"this matches to a corified verb reference ANYWHERE in the string";
{line} = args;
return match(line, "%$[a-z]+:[a-z]+[^(]");
.
@verb #97:"match_recycler_valid" this none this
@program #97:match_recycler_valid
":match_recycler_valid(STR line) => bool";
{line} = args;
return match(line, "^[ ]*if (%$recycler:valid");
.
;$code_scanner:display_issues($code_scanner:scan_for_issues($code_scanner, "scan_for_issues"))
If you have ANSI support you can edit :display_issues to add some colorizing.
It's great to have these warnings show up automatically after you @program a verb. However you must be careful when integrating the code scanner as you don't want to break your @program verb to the point where you can't @program a fix for it.
I recommend two things:
- Create a copy of your @program verb and test integration with that.
- Put the $code_scanner code inside a try/except just to be on the safe side.
Put it somewhere after the set_verb_code() builtin is executed.
Sindome has a $scheduler which is an alternative to forking, that lets you schedule things to happen at specific times, as opposed to forking in your code. Other MOO's have similar things. If you do not, you can remove the fork() check, if you don't find it useful.