{"id":859,"date":"2020-02-08T18:27:21","date_gmt":"2020-02-08T18:27:21","guid":{"rendered":"http:\/\/www.vidarholen.net\/contents\/blog\/?p=859"},"modified":"2020-02-09T06:30:56","modified_gmt":"2020-02-09T06:30:56","slug":"lessons-learned-from-writing-shellcheck-githubs-now-most-starred-haskell-project","status":"publish","type":"post","link":"https:\/\/www.vidarholen.net\/contents\/blog\/?p=859","title":{"rendered":"Lessons learned from writing ShellCheck, GitHub&#8217;s now most starred Haskell project"},"content":{"rendered":"\n<div class=\"wp-block-jetpack-markdown\"><p><a href=\"https:\/\/www.shellcheck.net\">ShellCheck<\/a> is a static analysis tool that\npoints out common problems and pitfalls in shell scripts.<\/p>\n<p>As of last weekend it appears to have become GitHub&#8217;s <a href=\"https:\/\/github.com\/search?l=Haskell&amp;q=stars%3A%3E100&amp;s=stars&amp;type=Repositories\">most starred Haskell\nrepository<\/a>,\nafter a mention in MIT SIPB&#8217;s <a href=\"https:\/\/sipb.mit.edu\/doc\/safe-shell\/\">Writing Safe Shell\nScripts<\/a> guide.<\/p>\n<p>While obviously a frivolous metric in a niche category, I like to interpret this\nas meaning that people are finding ShellCheck as useful as I find\n<a href=\"https:\/\/pandoc.org\/\">Pandoc<\/a>, the excellent universal document converter I use\nfor notes, blog posts and ShellCheck&#8217;s man page, and which held a firm grip on\nthe top spot for a long time.<\/p>\n<p>I am very happy and humbled that so many people are finding the project helpful\nand useful. The response has been incredibly and overwhelmingly positive.\nSeveral times per week I see mentions from people who tried it out, and it\neither solved their immediate problem, or it taught them something new and\ninteresting they didn&#8217;t know before.<\/p>\n<p>I started the project 8 years ago, and this seems like a good opportunity to\nshare some of the lessons learned along the way.<\/p>\n<h3>Quick History<\/h3>\n<p>ShellCheck is generally considered a shell script linter, but it actually\nstarted life in 2012 as an IRC bot (of all things!) on #bash@Freenode. It&#8217;s\nstill there and as active as ever.<\/p>\n<p>The channel is the home of the comprehensive and frequently cited <a href=\"https:\/\/mywiki.wooledge.org\/BashFAQ\">Wooledge\nBashFAQ<\/a>, plus an additional list of\n<a href=\"https:\/\/mywiki.wooledge.org\/BashPitfalls\">common pitfalls<\/a>. Between them, they\ncurrently cover 178 common questions about Bash and POSIX sh.<\/p>\n<p>Since no one ever reads the FAQ, an existing bot allowed regulars to e.g.\nanswer any problem regarding variations of <a href=\"http:\/\/mywiki.wooledge.org\/BashPitfalls#pf1\"><code>for file in `ls`<\/code><\/a> with a simple <code>!pf 1<\/code>, and\nlet a bot point the person in the right direction (the IRC equivalent of\nStackOverflow&#8217;s &quot;duplicate of&quot;).<\/p>\n<p>ShellCheck&#8217;s original purpose was essentially to find out how many of these\nFAQs could be classified automatically, without any human input.<\/p>\n<p>Due to this, ShellCheck was designed for different goals than most linters.<\/p>\n<ol>\n<li>It would only run on buggy scripts, because otherwise they wouldn&#8217;t have been posted.<\/li>\n<li>It would only run once, and should be as helpful as possible on the first pass.<\/li>\n<li>It would run on my machine, not on arbitrary user&#8217;s systems.<\/li>\n<\/ol>\n<p>This will become relevant.<\/p>\n<h3>On Haskell<\/h3>\n<p>Since ShellCheck was a hobby project that wasn&#8217;t intended to run on random\npeople&#8217;s machines, I could completely ignore popularity, familiarity, and\npracticality, and pick the language that was the most fun and interesting.<\/p>\n<p>That was, of course, Haskell.<\/p>\n<p>As anyone who looks at code will quickly conclude, ShellCheck was my first real\nproject in the language.<\/p>\n<p>Some things worked really well:<\/p>\n<ul>\n<li>QuickCheck has been beyond amazing. ShellCheck has 1500 unit tests just because they&#8217;re incredibly quick and convenient to write. It&#8217;s so good that I&#8217;m adding a subsection for it.<\/li>\n<li>Parsec is a joy to write parsers in. Initially I dreaded e.g. implementing backticks because they require recursively re-invoking the parser on escaped string data, but every time I faced such issues, they turned out to be much easier than expected.<\/li>\n<li>Haskell itself is a very comfy, productive language to write. It&#8217;s not at all arcane or restrictive as first impressions might have you believe. I&#8217;d take it over Java or C++ for most things.<\/li>\n<li>Haskell is surprisingly portable. I was shocked when I discovered that people were running ShellCheck natively on Windows without problems. ARM required a few code changes at the time, but wouldn&#8217;t have today.<\/li>\n<\/ul>\n<p>Some things didn&#8217;t work <em>as<\/em> well:<\/p>\n<ul>\n<li>Haskell has an undeniably high barrier to entry for the uninitiated, and ShellCheck&#8217;s target audience is not Haskell developers. I think this has seriously limited the scope and number of contributions.<\/li>\n<li>It&#8217;s easier to write than to run: it&#8217;s been hard to predict and control runtime performance. For example, many of ShellCheck&#8217;s check functions take an explicit &quot;params&quot; argument. Converting them to a cleaner ReaderT led to a 10% total run time regression, so I had to revert it. It makes me wonder about the speed penalty of code I designed better to begin with.<\/li>\n<li>Controlling memory usage is also hard. I dropped multithreading support because I simply couldn&#8217;t figure out the space leaks.<\/li>\n<li>For people not already invested in the ecosystem, the runtime dependencies can be 100MB+. ShellCheck is available as a standalone ~8MB executable, which took some work and is still comparatively large.<\/li>\n<li>The Haskell ecosystem moves and breaks very quickly. New changes would frequently break on older platform versions. Testing the default platform version of mainstream distros in VMs was slow and tedious. Fortunately, Docker came along to make it easy to automate per-distro testing, and Stack brought reproducible Haskell builds.<\/li>\n<\/ul>\n<p>If starting a new developer tooling project for a mainstream audience, I might\nchoose a more mainstream language. I&#8217;d also put serious consideration into how\nwell the language runs on a JSVM, since (love it or hate it) this would solve\na lot of distribution, integration, and portability issues.<\/p>\n<p>ShellCheck&#8217;s API is not very cohesive. If starting a new project in Haskell\ntoday, I would start out by implementing a dozen business logic functions in\nevery part of the system in my best case pseudocode. This would help me figure\nout the kind of DSL I want, and help me develop a more uniform API on a\nsuitable stack of monads.<\/p>\n<h4>Unit testing made fun and easy<\/h4>\n<p>ShellCheck is ~10k LoC, but has an additional 1.5k unit tests. I&#8217;m not a unit testing evangelist, 100% completionist or TDD fanatic: this simply happened by itself because writing tests was <em>so<\/em> quick and easy. Here&#8217;s an example check:<\/p>\n<pre><code>prop_checkSourceArgs1 = verify checkSourceArgs &quot;#!\/bin\/sh\\n. script arg&quot;\nprop_checkSourceArgs2 = verifyNot checkSourceArgs &quot;#!\/bin\/sh\\n. script&quot;\nprop_checkSourceArgs3 = verifyNot checkSourceArgs &quot;#!\/bin\/bash\\n. script arg&quot;\ncheckSourceArgs = CommandCheck (Exactly &quot;.&quot;) f\n  where\nf t = whenShell [Sh, Dash] $\n    case arguments t of\n\t(file:arg1:_) -&gt; warn (getId arg1) 2240 $\n\t    &quot;The dot command does not support arguments in sh\/dash. Set them as variables.&quot;\n\t_ -&gt; return ()\n<\/code><\/pre>\n<p>The <code>prop_..<\/code> lines are individual unit tests. Note in particular that:<\/p>\n<ul>\n<li>Each test simply specifies whether the given check emits a warning for a snippet. The boilerplate fits on the same line<\/li>\n<li>The test is in the same file and place as the function, so it doesn&#8217;t require any cross-referencing<\/li>\n<li>It doubles as a doc comment that explains what the function is expected to trigger on<\/li>\n<li><code>checkSourceArgs<\/code> is in OO terms an unexposed private method, but no OO BS was required to &quot;expose it for testing&quot;<\/li>\n<\/ul>\n<p>Even the parser has tests like this, where it can check whether a given\nfunction parses the given string cleanly or with warnings.<\/p>\n<p>QuickCheck is better known for its ability to generate test cases for\ninvariants, which ShellCheck makes some minimal use of, but even without that\nI&#8217;ve never had a better test writing experience in any previous project of any\nlanguage.<\/p>\n<h3>On writing a parser<\/h3>\n<p>ShellCheck was the first real parser I ever wrote. I&#8217;ve since taken up a day\njob as a compiler engineer, which helps to put a lot of it into perspective.<\/p>\n<p>My most important lessons would be:<\/p>\n<ul>\n<li>Be careful if your parser framework makes it too easy to backtrack. Look up good parser design. I naively wrote a character based parser function for each construct like <code>${..}<\/code>, <code>$(..)<\/code>, <code>$'..'<\/code>, etc, and now the parser has to backtrack a dozen times to try every possibility when it hits a <code>$<\/code>. With a tokenizer or a parser that read <code>$<\/code> followed by <code>{..}<\/code>, <code>(..)<\/code> etc, it would have been much faster &#8212; in fact, just grouping all the <code>$<\/code> constructs behind a lookahead decreased total checking time by 10%.<\/li>\n<li>Consistently use a tab stop of 1 for all column counts. This is what e.g. GCC does, and it makes life easier for everyone involved. ShellCheck used Parsec&#8217;s default of 8, which has been a source of alignment bugs and unnecessary conversions ever since.<\/li>\n<li>Record the full span and not just the start index of your tokens. Everyone loves squiggly lines under bad code. Also consider whether you want to capture comments and whitespace so you can turn the AST back into a script for autofixing. ShellCheck retrofitted end positions and emits autofixes as a series of text edits based on token spans, and it&#8217;s neither robust nor convenient.<\/li>\n<\/ul>\n<p>ShellCheck&#8217;s parser has historically also been, let&#8217;s say, &quot;pragmatic&quot;. For example, shell scripts are case sensitive, but ShellCheck accepted <code>While<\/code> in place of <code>while<\/code> for loops.<\/p>\n<p>This is generally considered heresy, but it originally made sense when ShellCheck needed to be as helpful as possible on the first try for a known buggy script. Neither ShellCheck nor a human would not point out that <code>While sleep 1; do date; done<\/code> has a misplaced <code>do<\/code> and <code>done<\/code>, but most linters would since <code>While<\/code> is not considered a valid start of a loop.<\/p>\n<p>These days it not as useful, since any spurious warnings about <code>do<\/code> would disappear when the user fixed the warning for <code>While<\/code> and reran ShellCheck.<\/p>\n<p>It also gets in the way for advanced users who e.g. write a function called <code>While<\/code> and capitalized it that way because they don&#8217;t want it treated as a shell keyword. ShellCheck has rightly received some critisism for focusing too much on newbie mistakes at the expense of noise for advanced users. This is an active area of development.<\/p>\n<p>If designed again, ShellCheck would parse more strictly according to spec, and instead make liberal use of lookaheads with pragmatic interpretations to emit warnings, even if it often resulted in a single useful warning at a time.<\/p>\n<h3>On writing a static analysis tool<\/h3>\n<p>I hadn&#8217;t really pondered, didn&#8217;t really use, and definitely hadn&#8217;t written any\nstatic analysis or linting tools before. The first versions of ShellCheck\ndidn&#8217;t even have error codes, just plain English text befitting an IRC bot.<\/p>\n<ul>\n<li>Supplement terse warnings with a wiki\/web page. ShellCheck&#8217;s wiki has a page for each warning, like <a href=\"https:\/\/github.com\/koalaman\/shellcheck\/wiki\/SC2162\">SC2162<\/a>. It has an example of code that triggers, an example of a fix, a mention of cases where it may not apply, and has especially received a lot of praise for having an educational rationale explaining why this is worth fixing.<\/li>\n<li>You&#8217;re not treading new ground. There are well studied algorithms for whatever you want to do. ShellCheck has some simplistic ad-hoc algorithms for e.g. variable liveness, which could and should have been implemented using robust and well known techniques.<\/li>\n<li>If designed today with 20\/20 hindsight, ShellCheck would have a plan to work with (or as) a <a href=\"https:\/\/microsoft.github.io\/language-server-protocol\/\">Language Server<\/a> to help with editor integrations.<\/li>\n<li>Include simple ways to suppress warnings. Since ShellCheck was originally intended for one-shot scenarios, this was an afterthought. It was then added on a statement level where the idea was that you could put special comments in front of a function, loop, or regular command, and it would apply to the entire thing. This has been an endless source of confusion (why can&#8217;t you put it in front of a <code>case<\/code> branch?), and should have been done on a per-line level instead.<\/li>\n<li>Give tests metadata so you can filter them. ShellCheck&#8217;s original checks were simple functions invoked for each AST node. Some of them only applied to certain shells, but would still be invoked thousands of times just to check that they don&#8217;t apply and return. Command specific checks would all duplicate and repeat the work of determining whether the current node was a command, and whether it was <em>the<\/em> command. Disabled checks were all still run, and their hard work simply filtered out afterwards. With more metadata, these could have been more intelligently applied.<\/li>\n<li>Gather a test corpus! Examining the diff between runs on a few thousand scripts has been invaluable in evaluating the potential, true\/false positive rate, and general correctness of checks.<\/li>\n<li>Track performance. I simply added <code>time<\/code> output to the aforementioned diff, and it stopped several space leaks and quadratic explosions.<\/li>\n<\/ul>\n<p>For a test corpus, I set up one script to scrape pastebin links from #bash@Freenode, and another to scrape scripts from trending GitHub projects.<\/p>\n<p>The pastebin links were more helpful because they exactly represented the types of scripts that ShellCheck wanted to check. However, though they&#8217;re generally simple and posted publically, I don&#8217;t actually have any rights to redistribute them, so I can&#8217;t really publish them to allow people to test their contributions.<\/p>\n<p>The GitHub scripts are easier to redistribute since there&#8217;s provenance and semi-structured licensing terms, but they&#8217;re generally also less buggy and therefore less useful (except for finding false positives).<\/p>\n<p>Today I would probably have tried parsing the <a href=\"https:\/\/archive.org\/details\/stackexchange\">Stack Exchange Data Dump<\/a> instead.<\/p>\n<p>Finally, ShellCheck is generally reluctant to read arbitrary files (e.g. requiring a\nflag <code>-x<\/code> to follow included scripts). This is obviously because it was\nfirst a hosted service on IRC and <a href=\"https:\/\/www.shellcheck.net\">web<\/a> before containerization was made simple,\nand not because this is in any way helpful or useful for a local linter.<\/p>\n<h3>On having a side project while working at a large company<\/h3>\n<p>I worked at Google when I started ShellCheck. They were good sports about\nit, let me run the project and keep the copyright, as long as I kept it\nentirely separate from my day job. I later joined Facebook, where the policies\nwere the same.<\/p>\n<p>Both companies independently discovered and adopted ShellCheck without my\ninput, and the lawyers stressed the same points:<\/p>\n<ul>\n<li>The company must not get, or appear to get, any special treatment because of you working there. For example, don&#8217;t prioritize bugs they find.<\/li>\n<li>Don&#8217;t contribute to anything related to the project internally. Not even if it&#8217;s work related. Not even if it&#8217;s not. Not even on your own time.<\/li>\n<li>If anyone assigns you a related internal task\/bug, reject it and tell them they&#8217;ll have to submit a FOSS bug report.<\/li>\n<\/ul>\n<p>And after discovering late in the interview process that Apple has a blanket ban on all programming related hobby projects:<\/p>\n<ul>\n<li>Ask any potential new employer about their side project policy early on<\/li>\n<\/ul>\n<h3>On the name &quot;ShellCheck&quot;<\/h3>\n<p>I just thought it was a descriptive name with a cute pun. I had no idea that a\nportion of the population would consistently read &quot;SpellCheck&quot; no matter\nhow many times they saw it. Sorry for the confusion!<\/p>\n<\/div>\n\n\n\n<p><\/p>\n","protected":false},"excerpt":{"rendered":"","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"closed","sticky":false,"template":"","format":"standard","meta":{"footnotes":"","jetpack_publicize_message":"","jetpack_is_tweetstorm":false,"jetpack_publicize_feature_enabled":true},"categories":[23],"tags":[46],"class_list":["post-859","post","type-post","status-publish","format-standard","hentry","category-programming","tag-shellcheck"],"jetpack_featured_media_url":"","_links":{"self":[{"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=\/wp\/v2\/posts\/859","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=859"}],"version-history":[{"count":4,"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=\/wp\/v2\/posts\/859\/revisions"}],"predecessor-version":[{"id":863,"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=\/wp\/v2\/posts\/859\/revisions\/863"}],"wp:attachment":[{"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=859"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=859"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.vidarholen.net\/contents\/blog\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=859"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}