-
-
Notifications
You must be signed in to change notification settings - Fork 82
/
Copy pathcontributing.html
332 lines (280 loc) · 19 KB
/
contributing.html
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
<!DOCTYPE HTML>
<html lang="en" class="light sidebar-visible" dir="ltr">
<head>
<!-- Book generated using mdBook -->
<meta charset="UTF-8">
<title>Contributing - selene Documentation</title>
<!-- Custom HTML head -->
<meta name="description" content="">
<meta name="viewport" content="width=device-width, initial-scale=1">
<meta name="theme-color" content="#ffffff">
<link rel="icon" href="favicon.svg">
<link rel="shortcut icon" href="favicon.png">
<link rel="stylesheet" href="css/variables.css">
<link rel="stylesheet" href="css/general.css">
<link rel="stylesheet" href="css/chrome.css">
<link rel="stylesheet" href="css/print.css" media="print">
<!-- Fonts -->
<link rel="stylesheet" href="FontAwesome/css/font-awesome.css">
<link rel="stylesheet" href="fonts/fonts.css">
<!-- Highlight.js Stylesheets -->
<link rel="stylesheet" href="highlight.css">
<link rel="stylesheet" href="tomorrow-night.css">
<link rel="stylesheet" href="ayu-highlight.css">
<!-- Custom theme stylesheets -->
<!-- Provide site root to javascript -->
<script>
var path_to_root = "";
var default_theme = window.matchMedia("(prefers-color-scheme: dark)").matches ? "navy" : "light";
</script>
<!-- Start loading toc.js asap -->
<script src="toc.js"></script>
</head>
<body>
<div id="body-container">
<!-- Work around some values being stored in localStorage wrapped in quotes -->
<script>
try {
var theme = localStorage.getItem('mdbook-theme');
var sidebar = localStorage.getItem('mdbook-sidebar');
if (theme.startsWith('"') && theme.endsWith('"')) {
localStorage.setItem('mdbook-theme', theme.slice(1, theme.length - 1));
}
if (sidebar.startsWith('"') && sidebar.endsWith('"')) {
localStorage.setItem('mdbook-sidebar', sidebar.slice(1, sidebar.length - 1));
}
} catch (e) { }
</script>
<!-- Set the theme before any content is loaded, prevents flash -->
<script>
var theme;
try { theme = localStorage.getItem('mdbook-theme'); } catch(e) { }
if (theme === null || theme === undefined) { theme = default_theme; }
const html = document.documentElement;
html.classList.remove('light')
html.classList.add(theme);
html.classList.add("js");
</script>
<input type="checkbox" id="sidebar-toggle-anchor" class="hidden">
<!-- Hide / unhide sidebar before it is displayed -->
<script>
var sidebar = null;
var sidebar_toggle = document.getElementById("sidebar-toggle-anchor");
if (document.body.clientWidth >= 1080) {
try { sidebar = localStorage.getItem('mdbook-sidebar'); } catch(e) { }
sidebar = sidebar || 'visible';
} else {
sidebar = 'hidden';
}
sidebar_toggle.checked = sidebar === 'visible';
html.classList.remove('sidebar-visible');
html.classList.add("sidebar-" + sidebar);
</script>
<nav id="sidebar" class="sidebar" aria-label="Table of contents">
<!-- populated by js -->
<mdbook-sidebar-scrollbox class="sidebar-scrollbox"></mdbook-sidebar-scrollbox>
<noscript>
<iframe class="sidebar-iframe-outer" src="toc.html"></iframe>
</noscript>
<div id="sidebar-resize-handle" class="sidebar-resize-handle">
<div class="sidebar-resize-indicator"></div>
</div>
</nav>
<div id="page-wrapper" class="page-wrapper">
<div class="page">
<div id="menu-bar-hover-placeholder"></div>
<div id="menu-bar" class="menu-bar sticky">
<div class="left-buttons">
<label id="sidebar-toggle" class="icon-button" for="sidebar-toggle-anchor" title="Toggle Table of Contents" aria-label="Toggle Table of Contents" aria-controls="sidebar">
<i class="fa fa-bars"></i>
</label>
<button id="theme-toggle" class="icon-button" type="button" title="Change theme" aria-label="Change theme" aria-haspopup="true" aria-expanded="false" aria-controls="theme-list">
<i class="fa fa-paint-brush"></i>
</button>
<ul id="theme-list" class="theme-popup" aria-label="Themes" role="menu">
<li role="none"><button role="menuitem" class="theme" id="light">Light</button></li>
<li role="none"><button role="menuitem" class="theme" id="rust">Rust</button></li>
<li role="none"><button role="menuitem" class="theme" id="coal">Coal</button></li>
<li role="none"><button role="menuitem" class="theme" id="navy">Navy</button></li>
<li role="none"><button role="menuitem" class="theme" id="ayu">Ayu</button></li>
</ul>
<button id="search-toggle" class="icon-button" type="button" title="Search. (Shortkey: s)" aria-label="Toggle Searchbar" aria-expanded="false" aria-keyshortcuts="S" aria-controls="searchbar">
<i class="fa fa-search"></i>
</button>
</div>
<h1 class="menu-title">selene Documentation</h1>
<div class="right-buttons">
<a href="print.html" title="Print this book" aria-label="Print this book">
<i id="print-button" class="fa fa-print"></i>
</a>
</div>
</div>
<div id="search-wrapper" class="hidden">
<form id="searchbar-outer" class="searchbar-outer">
<input type="search" id="searchbar" name="searchbar" placeholder="Search this book ..." aria-controls="searchresults-outer" aria-describedby="searchresults-header">
</form>
<div id="searchresults-outer" class="searchresults-outer hidden">
<div id="searchresults-header" class="searchresults-header"></div>
<ul id="searchresults">
</ul>
</div>
</div>
<!-- Apply ARIA attributes after the sidebar and the sidebar toggle button are added to the DOM -->
<script>
document.getElementById('sidebar-toggle').setAttribute('aria-expanded', sidebar === 'visible');
document.getElementById('sidebar').setAttribute('aria-hidden', sidebar !== 'visible');
Array.from(document.querySelectorAll('#sidebar a')).forEach(function(link) {
link.setAttribute('tabIndex', sidebar === 'visible' ? 0 : -1);
});
</script>
<div id="content" class="content">
<main>
<h1 id="contributing"><a class="header" href="#contributing">Contributing</a></h1>
<p>selene is written in Rust, so knowledge of the ecosystem is expected.</p>
<p>selene uses <a href="https://github.com/Kampfkarren/full-moon">Full Moon</a> to parse the Lua code losslessly, meaning whitespace and comments are preserved. You can read the full documentation for full-moon on its <a href="https://docs.rs/full_moon/latest/full_moon/">docs.rs</a> page.</p>
<p>TODO: Upload selene-lib on crates.io and link the docs.rs page for that as well as throughout the rest of this article.</p>
<h2 id="writing-a-lint"><a class="header" href="#writing-a-lint">Writing a lint</a></h2>
<p>In selene, lints are created in isolated modules. To start, create a file in <code>selene-lib/src/lints</code> with the name of your lint. In this example, we're going to call the lint <code>cool_lint.rs</code>.</p>
<p>Let's now understand what a lint consists of. selene takes lints in the form of structs that implement the <code>Lint</code> trait. The <code>Lint</code> trait expects:</p>
<ul>
<li>A <code>Config</code> associated type that defines what the configuration format is expected to be. Whatever you pass must be <a href="https://serde.rs/">deserializable</a>.</li>
<li>An <code>Error</code> associated type that implements <a href="https://doc.rust-lang.org/std/error/trait.Error.html"><code>std::error::Error</code></a>. This is used if configurations can be invalid (such as a parameter only being a number within a range). Most of the time, configurations cannot be invalid (other than deserializing errors, which are handled by selene), and so you can set this to <a href="https://doc.rust-lang.org/std/convert/enum.Infallible.html"><code>std::convert::Infallible</code></a>.</li>
<li>A <code>SEVERITY</code> constant which is either <code>Severity::Error</code> or <code>Severity::Warning</code>. Use <code>Error</code> if the code is positively impossible to be correct.</li>
<li>A <code>LINT_TYPE</code> constant which is either <code>Complexity</code>, <code>Correctness</code>, <code>Performance</code>, or <code>Style</code>. So far not used for anything.</li>
<li>A <code>new</code> function with the signature <code>fn new(config: Self::Config) -> Result<Self, Self::Error></code>. With the selene CLI, this is called once.</li>
<li>A <code>pass</code> function with the signature <code>fn pass(&self, ast: &full_moon::ast::Ast, context: &Context, ast_context: &AstContext) -> Vec<Diagnostic></code>. The <code>ast</code> argument is the full-moon representation of the code. The <code>context</code> argument provides optional additional information, such as the standard library being used. The <code>ast_context</code> argument provides context specific to that AST, such as its scopes. Any <code>Diagnostic</code> structs returned here are displayed to the user.</li>
</ul>
<p>For our purposes, we're going to write:</p>
<pre><code class="language-rs">use super::*;
use std::convert::Infallible;
struct CoolLint;
impl Lint for CoolLint {
type Config = ();
type Error = Infallible;
const SEVERITY: Severity = Severity::Warning;
const LINT_TYPE: LintType = LintType::Style;
fn new(_: Self::Config) -> Result<Self, Self::Error> {
Ok(CoolLint)
}
fn pass(&self, ast: &Ast, _: &Context, _: &AstContext) -> Vec<Diagnostic> {
unimplemented!()
}
}
</code></pre>
<p>The implementation of <code>pass</code> is completely up to you, but there are a few common patterns.</p>
<ul>
<li>Creating a visitor over the ast provided and creating diagnostics based off of that. See <a href="https://github.com/Kampfkarren/selene/blob/master/selene-lib/src/lints/divide_by_zero.rs"><code>divide_by_zero</code></a> and <a href="https://github.com/Kampfkarren/selene/blob/master/selene-lib/src/lints/suspicious_reverse_loop.rs"><code>suspicious_reverse_loop</code></a> for straight forward examples.</li>
<li>Using the <code>ScopeManager</code> struct to lint based off of usage of variables and references. See <a href="https://github.com/Kampfkarren/selene/blob/master/selene-lib/src/lints/shadowing.rs"><code>shadowing</code></a> and <a href="https://github.com/Kampfkarren/selene/blob/master/selene-lib/src/lints/global_usage.rs"><code>global_usage</code></a>.</li>
</ul>
<h3 id="getting-selene-to-recognize-the-new-lint"><a class="header" href="#getting-selene-to-recognize-the-new-lint">Getting selene to recognize the new lint</a></h3>
<p>Now that we have our lint, we have to make sure selene actually knows to use it. There are two places you need to update.</p>
<p>In selene-lib/src/lib.rs, search for <code>use_lints!</code>. You will see something such as:</p>
<pre><code class="language-rs">use_lints! {
almost_swapped: lints::almost_swapped::AlmostSwappedLint,
divide_by_zero: lints::divide_by_zero::DivideByZeroLint,
empty_if: lints::empty_if::EmptyIfLint,
...
}
</code></pre>
<p>Put your lint in this list (alphabetical order) as the following format:</p>
<pre><code>lint_name: lints::module_name::LintObject,
</code></pre>
<p>For us, this would be:</p>
<pre><code>cool_lint: lints::cool_lint::CoolLint,
</code></pre>
<p>Next, in <code>selene-lib/src/lints.rs</code>, search for <code>pub mod</code>, and you will see:</p>
<pre><code class="language-rs">pub mod almost_swapped;
pub mod divide_by_zero;
pub mod empty_if;
...
</code></pre>
<p>Put your module name in this list, also in alphabetical order.</p>
<pre><code class="language-rs">pub mod almost_swapped;
pub mod cool_lint;
pub mod divide_by_zero;
pub mod empty_if;
...
</code></pre>
<p>And we're done! You should be able to <code>cargo build --bin selene</code> and be able to use your new lint.</p>
<h3 id="writing-tests"><a class="header" href="#writing-tests">Writing tests</a></h3>
<p>The selene codebase uses tests extensively for lints. It means we never have to actually build the CLI tool in order to test, and we can make sure we don't have any regressions. <strong>Testing is required if you want to submit your lint to the selene codebase.</strong></p>
<p>To write a simple test, create a folder in <code>selene-lib/tests</code> with the name of your lint. Then, create as many <code>.lua</code> files as you want to test. These should contain both cases that do and do not lint. For our cases, we're going to assume our test is called <code>cool_lint.lua</code>.</p>
<p>Then, in your lint module, add at the bottom:</p>
<pre><code class="language-rs">#[cfg(test)]
mod tests {
use super::{super::test_util::test_lint, *};
#[test]
fn test_cool_lint() {
test_lint(
CoolLint::new(()).unwrap(),
"cool_lint",
"cool_lint",
);
}
}
</code></pre>
<p>Let's discuss what this code means, assuming you're familiar with <a href="https://doc.rust-lang.org/book/ch11-00-testing.html">the way tests are written and performed in Rust</a>.</p>
<p>The <code>test_lint</code> function is the easiest way to test that a lint works. It'll search the source code we made before, run selene on it (only your lint), and check its results with the existing <code>[filename].stderr</code> file, or create one if it does not yet exist.</p>
<p>The first argument is the lint object to use. <code>CoolLint::new(())</code> just means "create <code>CoolLint</code> with a configuration of <code>()</code>". If your lint specifies a configuration, this will instead be something such as <code>CoolLintConfig::default()</code> or whatever you're specifically testing.</p>
<p>The <code>.unwrap()</code> is just because <code>CoolLint::new</code> returns a <code>Result</code>. If you want to test configuration errors, you can avoid <code>test_lint</code> altogether and just test <code>CoolLint::new(...).is_err()</code> directly.</p>
<p>The first <code>"cool_lint"</code> is the name of the folder we created. The second <code>"cool_lint"</code> is the name of the <em>Lua file</em> we created.</p>
<p>Now, just run <code>cargo test</code>, and a <code>.stderr</code> file will be automatically generated! You can manipulate it however you see fit as well as modifying your lint, and so long as the file is there, selene will make sure that its accurate.</p>
<p>Optionally, you can add a <code>.std.toml</code> with the same name as the test next to the lua file, where you can specify a custom <a href="./usage/std.html">standard library</a> to use. If you do not, the Lua 5.1 standard library will be used.</p>
<h3 id="documenting-it"><a class="header" href="#documenting-it">Documenting it</a></h3>
<p>This step is only if you are contributing to the selene codebase, and not just writing personal lints (though I'm sure your other programmers would love if you did this).</p>
<p>To document a new lint, edit <code>docs/src/SUMMARY.md</code>, and add your lint to the table of contents along the rest. As with everything else, make sure it's in alphabetical order.</p>
<p>Then, edit the markdown file it creates (if you have <code>mdbook serve</code> on, it'll create it for you), and write it in this format:</p>
<pre><code># lint_name
## What it does
Explain what your lint does, simply.
## Why this is bad
Explain why a user would want to lint this.
## Configuration
Specify any configuration if it exists.
## Example
```lua
-- Bad code here
```
...should be written as...
```lua
-- Good code here
```
## Remarks
If there's anything else a user should know when using this lint, write it here.
</code></pre>
<p>This isn't a strict format, and you can mess with it as appropriate. For example, <code>standard_library</code> does not have a "Why this is bad" section as not only is it a very encompassing lint, but it should be fairly obvious. Many lints don't specify a "...should be written as..." as it is either something with various potential fixes (such as <a href="./lints/global_usage.html"><code>global_usage</code></a>) or because the "good code" is just removing parts entirely (such as <a href="./lints/unbalanced_assignments.html"><code>unbalanced_assignments</code></a>).</p>
</main>
<nav class="nav-wrapper" aria-label="Page navigation">
<!-- Mobile navigation buttons -->
<a rel="prev" href="roblox.html" class="mobile-nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
<i class="fa fa-angle-left"></i>
</a>
<a rel="next prefetch" href="lints/index.html" class="mobile-nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
<i class="fa fa-angle-right"></i>
</a>
<div style="clear: both"></div>
</nav>
</div>
</div>
<nav class="nav-wide-wrapper" aria-label="Page navigation">
<a rel="prev" href="roblox.html" class="nav-chapters previous" title="Previous chapter" aria-label="Previous chapter" aria-keyshortcuts="Left">
<i class="fa fa-angle-left"></i>
</a>
<a rel="next prefetch" href="lints/index.html" class="nav-chapters next" title="Next chapter" aria-label="Next chapter" aria-keyshortcuts="Right">
<i class="fa fa-angle-right"></i>
</a>
</nav>
</div>
<script>
window.playground_copyable = true;
</script>
<script src="elasticlunr.min.js"></script>
<script src="mark.min.js"></script>
<script src="searcher.js"></script>
<script src="clipboard.min.js"></script>
<script src="highlight.js"></script>
<script src="book.js"></script>
<!-- Custom JS scripts -->
</div>
</body>
</html>