We’ve All Got Issues
Who among us doesn't have issues, amirite? Let's just take a moment and acknowledge this fact and I think we can always be a bit more honest and understanding of all of our unique issues and the various idiosyncrasies we exhibit. While we can all offer understanding and grace to each other, some of the issues we face can be important to address quickly, and some we can perhaps choose to work on when the time is right.
...and so it is with our code. Issues, bugs, and other sub-optimal constructs crop up all the time in any serious codebase. Many of you are already aware of the features of the Code Analyzer in MATLAB that do a great job of alerting you to your issues directly in the MATLAB editor. However, sometimes issues still can creep into a code base through a number of subtle means, even including simply failing to act when a helpful editor is trying to tell me I have a problem in my code.
However, in R2022b there was a great improvement to the programmatic interface for identifying and addressing with these code issues. In fact, this new interface is itself a new function called codeIssues!
This new API for the code analyzer is in a word - powerful. Let's explore how you can now use this to build quality into your project. Starting with another look at our standard mass-spring-damper mini-codebase. Most recently we talked about this when describing the new MATLAB build tool. This code base is pretty simple. It includes a simulator, a function to return some design constants (spring constant, damping coefficient, etc.), and some tests.
To get started on a codebase like this, just call codeIssues on the folder you want to analyze:
You can see that with just a simple call to codeIssues you can quickly get an overview of all the details a static analyzer dreams of. You can easily dig into the files that were analyzed, the configuration, and the very handy table of the issues found, as well as any issues that have been suppressed through suppression pragmas in the editor. If you are in MATLAB you can even click on each issue to get right to it in the MATLAB editor where it can be fixed or suppressed if needed.
Now with this beautiful API at our fingertips, and with the build tool to boot, we can lock down our code in a much more robust, automated way. We can start roughly where we left off at the end of our build tool post with the following buildfile with a mex and a test task:
function plan = buildfile plan = buildplan(localfunctions); plan("test").Dependencies = "mex"; plan.DefaultTasks = "test"; end function mexTask(~) % Compile mex files mex mex/convec.c -outdir toolbox/; end function testTask(~) % Run the unit tests results = runtests; disp(results); assertSuccess(results); end
Let's go ahead and add a "codeIssues" task to this build by creating a new local function called codeIssuesTask:
function codeIssuesTask(~) % Get all the issues under toolbox allIssues = codeIssues("toolbox"); % Assert that no errors creep into the codebase errorIdx = allIssues.Issues.Severity == "error"; errors = allIssues.Issues(errorIdx,:); otherIssues = allIssues.Issues(~errorIdx,:); if ~isempty(errors) disp("Found critical errors in code:"); disp(errors); else disp("No critical errors found."); end % Display all the other issues if ~isempty(otherIssues) disp("Other Issues:") disp(otherIssues); else disp("No other issues found either. (wow, good for you!)") end assert(isempty(errors)); end
This is quite simple, we just want to find all the issues under the "toolbox" folder and throw an assertion error if any of them are of Severity "error". This is just about the quickest win you can apply to a code base to build quality into it. This can find syntax and other errors statically, without even writing or running a single test. There really is no reason at all we shouldn't apply this task to every project. It costs virtually nothing and can be remarkably efficient at finding bugs. On that note, let's add it as a default task in our buildfile:
plan.DefaultTasks = ["codeIssues" "test"];
...and with that we now have this check built right into our standard development process. To show this let's first put a file with an error into the code base and then we can call the build tool:
function syntaxError disp("Forgot the closing parentheses!"
copyfile .changes/syntaxError.m toolbox/syntaxError.m try buildtool catch ex disp(ex.getReport("basic")); end
** Starting codeIssues Failed! Found critical errors in code: Location Severity Description CheckID LineStart LineEnd ColumnStart ColumnEnd FullFilename _______________ ________ ______________________________________________________________________________ _______ _________ _______ ___________ _________ _______________________________________________________________________________________________ "syntaxError.m" error "A '(' might be missing a closing ')', causing invalid syntax at end of line." EOLPAR 3 3 5 5 "/Users/acampbel/Library/CloudStorage/OneDrive-MathWorks/repos/msd_blog2/toolbox/syntaxError.m" Other Issues: Location Severity Description CheckID LineStart LineEnd ColumnStart ColumnEnd FullFilename __________________________ ________ _____________________________________________ _______ _________ _______ ___________ _________ __________________________________________________________________________________________________________ "springMassDamperDesign.m" warning "Value assigned to variable might be unused." NASGU 4 4 3 3 "/Users/acampbel/Library/CloudStorage/OneDrive-MathWorks/repos/msd_blog2/toolbox/springMassDamperDesign.m" "springMassDamperDesign.m" warning "Value assigned to variable might be unused." NASGU 6 6 3 3 "/Users/acampbel/Library/CloudStorage/OneDrive-MathWorks/repos/msd_blog2/toolbox/springMassDamperDesign.m" ## ----------------------------------------------------------------------------- ## Error using assert ## Assertion failed. ## ## Error in buildfile>codeIssuesTask (line 67) ## assert(isempty(errors)); ## ----------------------------------------------------------------------------- ** Failed codeIssues Error using buildtool Build failed.
The build tool has done its part in stopping our development process in its tracks when it sees we have some important syntax errors to address.
Let's remove that bunk error so we can see our "codeIssues" task complete successfully. When we do this we also successfully execute the other "mex" and "test" tasks to complete the whole workflow.
delete toolbox/syntaxError.m
buildtool
** Starting codeIssues No critical errors found. Other Issues: Location Severity Description CheckID LineStart LineEnd ColumnStart ColumnEnd FullFilename __________________________ ________ _____________________________________________ _______ _________ _______ ___________ _________ __________________________________________________________________________________________________________ "springMassDamperDesign.m" warning "Value assigned to variable might be unused." NASGU 4 4 3 3 "/Users/acampbel/Library/CloudStorage/OneDrive-MathWorks/repos/msd_blog2/toolbox/springMassDamperDesign.m" "springMassDamperDesign.m" warning "Value assigned to variable might be unused." NASGU 6 6 3 3 "/Users/acampbel/Library/CloudStorage/OneDrive-MathWorks/repos/msd_blog2/toolbox/springMassDamperDesign.m" ** Finished codeIssues ** Starting mex Building with 'Xcode with Clang'. MEX completed successfully. ** Finished mex ** Starting test Setting up ProjectFixture Done setting up ProjectFixture: Project 'msd' is already loaded. Set up is not required. __________ Running convecTest . Done convecTest __________ Running designTest ... Done designTest __________ Tearing down ProjectFixture Done tearing down ProjectFixture: Teardown is not required. __________ 1×4 TestResult array with properties: Name Passed Failed Incomplete Duration Details Totals: 4 Passed, 0 Failed, 0 Incomplete. 0.22349 seconds testing time. ** Finished test
One last thing
Now we have you all setup nice and cozy with the protection that static analysis gives you. However, while we fail on static analysis errors, I am still uncomfortable with how easy it is to continue to add constructs to my code that result in static analysis warnings, which often point to real problems in your program. We could also fail the build on warnings if we'd like, but I didn't want to start with that idea out of the gate.
It is pretty clear that we want this protection with full-on-bonafide errors, which are almost always bugs. We run into a problem though when a code base already has an inventory of warnings. It would be fantastic to go through that inventory and fix all of those warnings as well. In fact, the new code analysis tooling makes that very easy in many cases! However, you may not be up for this right now. Your code base may be large, and you may want or need to invest a bit more time into this activity. So our first crack at this failed the build only for issues with an "error" Severity.
However, if you know me you know I like to sneak one last thing in. What if we accepted all of our current warnings in the codebase, but wanted to lock down our code base such that we are protected from introducing new warnings? To me this sounds like a great idea. We can then ratchet down our warnings by preventing inflow of new warnings and can remove existing warnings over time through interacting with the codebase. How can we do this? We can leverage the power of the codeIssues programmatic API!
We can do this by capturing and saving our existing warnings to a baseline of known issues. As MATLAB tables, theses issues are in a nice representation to save in a *.csv or *.xlsx file. Saving them in this format makes it really easy to tweak them, open them outside of MATLAB, or even remove issues that have been fixed.
To do this we just need to make a couple tweaks to the issues table. We need to overwrite the Location variable with relative paths to the files, remove the FullFilename variable, and make a quick datatype tweak to allow for nice CSV'ing. The relative filename adjustment is important because we want to be able to compare these results across different machines and the full path is likely to differ across environments. Such environments include the desktops of individual engineers as well as different build agents in a CI system.
That function looks as follows:
function theTable = preprocessIssues(theTable) % Make an issues table conducive for baselining via a few small tweaks % Overwrite the location field with relative paths, and remove absolute paths basePath = string(pwd) + filesep; theTable.Location = erase(theTable.FullFilename, basePath); theTable.Properties.VariableNames{"Location"} = 'RelativeFilename'; theTable.FullFilename = []; % Convert the Severity to categorical, which serializes nicely to string theTable.Severity = categorical(theTable.Severity); end
...and now with this function we can create a new task in the buildfile to generate a new baseline:
function captureWarningsBaselineTask(~) % Captures the current codeIssues warnings and creates a baseline csv file allIssues = codeIssues("toolbox"); warningIdx = allIssues.Issues.Severity == "warning"; warnings = allIssues.Issues(warningIdx,:); warnings = preprocessIssues(warnings); if ~isempty(warnings) disp("Saving a new ""knownIssues.csv"" baseline file for " + height(warnings) + " code warnings") writetable(warnings, "knownIssues.csv"); else disp("No warnings to create a baseline for") end end
Let's do it!
buildtool captureWarningsBaseline
** Starting captureWarningsBaseline Saving a new "knownIssues.csv" baseline file for 2 code warnings ** Finished captureWarningsBaseline
Great I now see the csv files. We can take a peek:
type knownIssues.csv
RelativeFilename,Severity,Description,CheckID,LineStart,LineEnd,ColumnStart,ColumnEnd toolbox/springMassDamperDesign.m,warning,Value assigned to variable might be unused.,NASGU,4,4,3,3 toolbox/springMassDamperDesign.m,warning,Value assigned to variable might be unused.,NASGU,6,6,3,3
Beautiful. In this case we just have two minor warnings that I don't want to look into quite yet. However, now we can adjust the "codeIssues" task to prevent me from introducing anything new:
function codeIssuesTask(~) % Get all the issues under toolbox allIssues = codeIssues("toolbox"); % Assert that no errors creep into the codebase errorIdx = allIssues.Issues.Severity == "error"; errors = allIssues.Issues(errorIdx,:); otherIssues = allIssues.Issues(~errorIdx,:); if ~isempty(errors) disp("Failed! Found critical errors in code:"); disp(errors); else disp("No critical errors found."); end % Load the known warnings baseline newWarnings = []; if isfile("knownIssues.csv") otherIssues = preprocessIssues(otherIssues); % Load the baseline file opts = detectImportOptions("knownIssues.csv"); types = varfun(@class, otherIssues,"OutputFormat","cell"); opts.VariableTypes = types; knownIssues = readtable("knownIssues.csv",opts); % Find the new warnings by subtracting the known issues in the baseline otherIssues = setdiff(otherIssues, knownIssues); newWarningIdx = otherIssues.Severity == "warning"; newWarnings = otherIssues(newWarningIdx,:); if ~isempty(newWarnings) disp("Failed! Found new warnings in code:"); disp(newWarnings); else disp("No new warnings found."); end otherIssues = [knownIssues; otherIssues(~newWarningIdx,:)]; end % Display all the other issues if ~isempty(otherIssues) disp("Other Issues:") disp(otherIssues); else disp("No other issues found either. (wow, good for you!)") end assert(isempty(errors)); assert(isempty(newWarnings)); end
This now loads the issues and does a setdiff to ignore those that are already known and captured in our baseline CSV file. This way, at least from now on I won't introduce any new warnings to the code base. It can only get better from here. Also, if I change some file that has an existing warning, there is a decent chance that my build tooling is going to yell at me because the existing warning is slightly different. For example it might be on a different line due to changes made in the file.
If this happens, great! Make me clean up or suppress the warning while I have the file open and modified. That's a feature not a bug. Worst case scenario, I can always capture a new baseline if I really can't look into it immediately, but I love this approach to help me clean my code through the process.
What does this look like? Let's add a file to the codebase with a new warning:
function codeWarning anUnusedVariable = "unused";
...and invoke the build:
copyfile .changes/codeWarning.m toolbox/codeWarning.m try buildtool catch ex disp(ex.getReport("basic")); end delete toolbox/codeWarning.m
** Starting codeIssues No critical errors found. Failed! Found new warnings in code: RelativeFilename Severity Description CheckID LineStart LineEnd ColumnStart ColumnEnd _______________________ ________ _____________________________________________ _______ _________ _______ ___________ _________ "toolbox/codeWarning.m" warning "Value assigned to variable might be unused." NASGU 3 3 1 16 Other Issues: RelativeFilename Severity Description CheckID LineStart LineEnd ColumnStart ColumnEnd __________________________________ ________ _____________________________________________ _______ _________ _______ ___________ _________ "toolbox/springMassDamperDesign.m" warning "Value assigned to variable might be unused." NASGU 4 4 3 3 "toolbox/springMassDamperDesign.m" warning "Value assigned to variable might be unused." NASGU 6 6 3 3 ## ----------------------------------------------------------------------------- ## Error using assert ## Assertion failed. ## ## Error in buildfile>codeIssuesTask (line 68) ## assert(isempty(newWarnings)); ## ----------------------------------------------------------------------------- ** Failed codeIssues Error using buildtool Build failed.
Love it! I am now protected from myself. I can leverage this in my standard toolbox development process to help ensure that over time my code only gets better, not worse. You could also imagine tweaking this to fail or otherwise notify when a warning goes away from the known issues so that we have some pressure to help ensure the lockdown gets tighter and tighter as time goes on. For reference, here is the final buildfile used for this workflow discussed today:
function plan = buildfile plan = buildplan(localfunctions); plan("test").Dependencies = "mex"; plan.DefaultTasks = ["codeIssues" "test"]; end function mexTask(~) % Compile mex files mex mex/convec.c -outdir toolbox/; end function testTask(~) % Run the unit tests results = runtests; disp(results); assertSuccess(results); end function codeIssuesTask(~) % Get all the issues under toolbox allIssues = codeIssues("toolbox"); % Assert that no errors creep into the codebase errorIdx = allIssues.Issues.Severity == "error"; errors = allIssues.Issues(errorIdx,:); otherIssues = allIssues.Issues(~errorIdx,:); if ~isempty(errors) disp("Failed! Found critical errors in code:"); disp(errors); else disp("No critical errors found."); end % Load the known warnings baseline newWarnings = []; if isfile("knownIssues.csv") otherIssues = preprocessIssues(otherIssues); opts = detectImportOptions("knownIssues.csv"); types = varfun(@class, otherIssues,"OutputFormat","cell"); opts.VariableTypes = types; knownIssues = readtable("knownIssues.csv",opts); otherIssues = setdiff(otherIssues, knownIssues); newWarningIdx = otherIssues.Severity == "warning"; newWarnings = otherIssues(newWarningIdx,:); if ~isempty(newWarnings) disp("Failed! Found new warnings in code:"); disp(newWarnings); else disp("No new warnings found."); end otherIssues = [knownIssues; otherIssues(~newWarningIdx,:)]; end % Display all the other issues if ~isempty(otherIssues) disp("Other Issues:") disp(otherIssues); else disp("No other issues found either. (wow, good for you!)") end assert(isempty(errors)); assert(isempty(newWarnings)); end function captureWarningsBaselineTask(~) % Captures the current codeIssues warnings and creates a baseline csv file allIssues = codeIssues("toolbox"); warningIdx = allIssues.Issues.Severity == "warning"; warnings = allIssues.Issues(warningIdx,:); warnings = preprocessIssues(warnings); if ~isempty(warnings) disp("Saving a new ""knownIssues.csv"" baseline file for " + height(warnings) + " code warnings") writetable(warnings, "knownIssues.csv"); else disp("No warnings to create a baseline for") end end function theTable = preprocessIssues(theTable) % Make an issues table conducive for baselining via a few small tweaks % Overwrite the location field with relative paths, and remove absolute paths basePath = string(pwd) + filesep; theTable.Location = erase(theTable.FullFilename, basePath); theTable.Properties.VariableNames{"Location"} = 'RelativeFilename'; theTable.FullFilename = []; % Convert the Severity to categorical, which serializes nicely to string theTable.Severity = categorical(theTable.Severity); end
There you have it, a clean API for MATLAB's code analysis and a standard way to include this in the development process using the build tool. I can practically feel the quality getting higher!
Folks, there is so much to blog about in the next little while. There's more to discuss here on how we can leverage new and improving tools to develop clean build and test pipelines for your MATLAB projects. Also, I am so excited for some really fantastic progress we will be able to share shortly. Buckle in, we are gonna be talking about high quality test and automation developments for a bit here on this blog. Chime in with your insights, tools, and workflows you use as you develop your professional MATLAB projects.
评论
要发表评论,请点击 此处 登录到您的 MathWorks 帐户或创建一个新帐户。