lute lint: default lint rules (#624)

Implement behavior for `lute lint` to use default lint rules if none are
specified. After discussing with @~Vighnesh-V, we've decided to keep the
example rules in `examples` as-is, but keep the actual lint rules used
inside `lute/cli/commands/lint/rules`, which will also hold any lint
rules added in the future.

Also deleted the `linter.luau` example in case people go thinking that's
the linter instead.

Author: skberkeley

examples/linter.luau

@@ -1,3 +1,5 @@
+-- Check out lute/cli/commands/lint/rules for the default rules used by lute lint!
+
 local lintTypes = require("@commands/lint/types")
 local path = require("@std/path")
 local query = require("@std/syntax/query")

examples/lints/almost_swapped.luau

@@ -1,3 +1,5 @@
+-- Check out lute/cli/commands/lint/rules for the default rules used by lute lint!
+
 local lintTypes = require("@commands/lint/types")
 local path = require("@std/path")
 local query = require("@std/syntax/query")

examples/lints/divide_by_zero.luau

@@ -7,16 +7,20 @@ local syntax = require("@std/syntax")
 local tableext = require("@std/tableext")
 local types = require("@self/types")
 
+local DEFAULT_RULES = { "almost_swapped", "divide_by_zero" }
 local USAGE = "Usage: lute lint [OPTIONS] [...PATHS]"
 
 local function printHelp()
 	print(USAGE)
 	print([[
-Lint the specified Luau file using the specified lint rule(s).
+Lint the specified Luau file using the specified lint rule(s) or using the default rules.
 
 OPTIONS:
   -h, --help              Show this help message
-  -r, --rules [RULE]      Path to a single lint rule or a folder containing lint rules. If a folder is provided, any subfolders containing init.luau files will be treated as modules exporting lint rules, while all other .luau files will be treated as individual lint rules.
+  -r, --rules [RULE]      Path to a single lint rule or a folder containing lint rules. If a folder 
+  						  is provided, any subfolders containing init.luau files will be treated as
+						  modules exporting lint rules, while all other .luau files will be treated 
+						  as individual lint rules. If unspecified, the default lint rules are used.
 
 PATHS:
   Path(s) to the Luau file(s) or folders containing Luau files to be linted. Only files with .luau or .lua extensions will be linted.
@@ -27,17 +31,21 @@ EXAMPLES:
 ]])
 end
 
-local function loadLintRule(path: string): types.LintRule
-	local loaded = luau.loadbypath(path)
-	assert(loaded, `{path} failed to require`)
-	assert(typeof(loaded) == "table", `{path} must return a table`)
-	assert(typeof(loaded.name) == "string", `{path} must return a table with a 'name' string property`)
+local function assertIsLintRule(rule: unknown, path: string)
+	assert(rule, `{path} failed to require`)
+	assert(typeof(rule) == "table", `{path} must return a table`)
+	assert(typeof(rule.name) == "string", `{path} must return a table with a 'name' string property`)
 	assert(
-		typeof(loaded.lint) == "function", -- LUAUFIX: type error on loaded.lint because loaded has been refined to { read name: string }
+		typeof(rule.lint) == "function", -- LUAUFIX: type error on loaded.lint because loaded has been refined to { read name: string }
 		`{path} must return a table with a 'lint' function property`
 	)
+end
+
+local function loadLintRule(path: string): types.LintRule
+	local loaded = luau.loadbypath(path)
+	assertIsLintRule(loaded, path)
 
-	return loaded :: types.LintRule -- We have to cast because we can only assert that loaded.lint is the top function type, rather than a more specific function type like (ast: syntax.AstStatBlock, sourcepath: path.path) -> { LintViolation }
+	return loaded -- Typecast isn't needed because loaded is refined to any bc of type error in assertIsLintRule
 end
 
 local function loadLintRules(path: string): { types.LintRule }
@@ -70,6 +78,17 @@ local function loadLintRules(path: string): { types.LintRule }
 	return rules
 end
 
+local function loadDefaultRules(): { types.LintRule }
+	local rules = {}
+	for _, ruleName in DEFAULT_RULES do
+		local path = `@self/rules/{ruleName}`
+		local rule = require(path)
+		assertIsLintRule(rule, path)
+		table.insert(rules, rule)
+	end
+	return rules
+end
+
 local function lintFile(inputFilePath: pathLib.path, lintRules: { types.LintRule }): { types.LintViolation }
 	print(`Reading input file '{inputFilePath}'`)
 	local fileContent = fs.readfiletostring(inputFilePath)
@@ -104,7 +123,7 @@ local function main(...: string)
 
 	args:add("rules", "option", { help = "Linting rule(s) to apply", aliases = { "r" } })
 	args:add("help", "flag", { help = "Show help message", aliases = { "h" } })
-	-- TODO: handle multiple rules, multiple input files, ignore blobs, json output option
+	-- TODO: ignore blobs, json output option
 
 	args:parse({ ... })
 
@@ -113,21 +132,21 @@ local function main(...: string)
 		return
 	end
 
-	local rulePath = args:get("rules")
-	if rulePath == nil then
-		-- TODO: support default rules
-		print("Error: No lint rules specified.\n\n" .. USAGE .. "\nUse --help for more information.")
-		return
-	end
-
 	local inputFiles = args:forwarded()
 	if inputFiles == nil or #inputFiles == 0 then
 		print("Error: No input files specified.\n\n" .. USAGE .. "\nUse --help for more information.")
 		return
 	end
 
-	print(`Loading lint rule(s) from '{rulePath}'`)
-	local lintRules = loadLintRules(rulePath)
+	local lintRules: { types.LintRule }
+	local rulePath = args:get("rules")
+	if rulePath == nil then
+		print("Using default lint rules.")
+		lintRules = loadDefaultRules()
+	else
+		print(`Loading lint rule(s) from '{rulePath}'`)
+		lintRules = loadLintRules(rulePath)
+	end
 
 	local allViolations = {}
 

lute/cli/commands/lint/init.luau

@@ -0,0 +1 @@
+To add a new default lint rule to `lute lint`, create a module or luau file defining the rule in this folder. Then, add the name of the rule to the DEFAULT_RULES constant in `lint/init.luau`, and add a test for the rule in `tests/cli/lint.test.luau`. The expected type of a lint rule is specified in `lint/types.luau`. 

lute/cli/commands/lint/rules/README.md

@@ -0,0 +1,105 @@
+local lintTypes = require("../types")
+local path = require("@std/path")
+local query = require("@std/syntax/query")
+local syntax = require("@std/syntax")
+local utils = require("@std/syntax/utils")
+
+local name = "almost_swapped"
+local message = "This looks like a failed attempt to swap."
+
+local compFuncs = {}
+
+function compFuncs.exprLocalsSame(a: syntax.AstExprLocal, b: syntax.AstExprLocal): boolean
+	return a["local"] == b["local"]
+end
+
+function compFuncs.exprGlobalsSame(a: syntax.AstExprGlobal, b: syntax.AstExprGlobal): boolean
+	return a.name.text == b.name.text
+end
+
+function compFuncs.exprIndexNamesSame(a: syntax.AstExprIndexName, b: syntax.AstExprIndexName): boolean
+	if a.index.text ~= b.index.text then
+		return false
+	end
+
+	return compFuncs.refExprsSame(a.expression, b.expression)
+end
+
+function compFuncs.exprIndexExprsSame(a: syntax.AstExprIndexExpr, b: syntax.AstExprIndexExpr): boolean
+	if a.index.tag == "string" and b.index.tag == "string" then
+		if a.index.text ~= b.index.text then
+			return false
+		else
+			return compFuncs.refExprsSame(a.expression, b.expression)
+		end
+	else
+		return compFuncs.refExprsSame(a.expression, b.expression) and compFuncs.refExprsSame(a.index, b.index)
+	end
+end
+
+function compFuncs.refExprsSame(a: syntax.AstExpr, b: syntax.AstExpr): boolean
+	if a.tag ~= b.tag then
+		return false
+	end
+
+	if a.tag == "local" then
+		return compFuncs.exprLocalsSame(a, b :: syntax.AstExprLocal)
+	elseif a.tag == "global" then
+		return compFuncs.exprGlobalsSame(a, b :: syntax.AstExprGlobal)
+	elseif a.tag == "indexname" then
+		return compFuncs.exprIndexNamesSame(a, b :: syntax.AstExprIndexName)
+	elseif a.tag == "index" then
+		return compFuncs.exprIndexExprsSame(a, b :: syntax.AstExprIndexExpr)
+	else
+		return false
+	end
+end
+
+-- Report instances of attempted swaps like:
+-- a = b; b = a
+local function lint(ast: syntax.AstStatBlock, sourcepath: path.path): { lintTypes.LintViolation }
+	local violations = {}
+
+	local nodes = query.findallfromroot(ast, utils.isStatBlock).nodes
+
+	for _, block in nodes do
+		for i = 1, #block.statements - 1 do
+			local currStat = block.statements[i]
+			if currStat.tag ~= "assign" or #currStat.values ~= 1 or #currStat.variables ~= 1 then
+				continue
+			end
+
+			local nextStat = block.statements[i + 1]
+			if nextStat.tag ~= "assign" or #nextStat.values ~= 1 or #nextStat.variables ~= 1 then
+				continue
+			end
+
+			local currVar, currVal = currStat.variables[1].node, currStat.values[1].node
+			local nextVar, nextVal = nextStat.variables[1].node, nextStat.values[1].node
+
+			if compFuncs.refExprsSame(currVar, nextVal) and compFuncs.refExprsSame(nextVar, currVal) then
+				table.insert(violations, { -- LUAUFIX: severity isn't inferred as a singleton, so table.insert is mad
+					lintname = name,
+					location = syntax.span.create({
+						beginline = currStat.location.beginline,
+						begincolumn = currStat.location.begincolumn,
+						endline = nextStat.location.endline,
+						endcolumn = nextStat.location.endcolumn,
+					}),
+					message = message,
+					severity = "warning",
+					sourcepath = sourcepath,
+				})
+			end
+		end
+	end
+
+	return violations
+end
+
+local rule: lintTypes.LintRule = {
+	name = name,
+	lint = lint,
+}
+
+return table.freeze(rule)

lute/cli/commands/lint/rules/almost_swapped.luau

@@ -0,0 +1,39 @@
+local lintTypes = require("../types")
+local path = require("@std/path")
+local query = require("@std/syntax/query")
+local syntax = require("@std/syntax")
+local utils = require("@std/syntax/utils")
+
+local name = "divide_by_zero"
+local message = "Division by zero detected."
+
+local function lint(ast: syntax.AstStatBlock, sourcepath: path.path): { lintTypes.LintViolation }
+	return query
+		.findallfromroot(ast, utils.isExprBinary)
+		:filter(function(bin)
+			return bin.operator.text == "/" or bin.operator.text == "//" or bin.operator.text == "%"
+		end)
+		:filter(function(bin)
+			return bin.rhsoperand.kind == "expr" and bin.rhsoperand.tag == "number" and bin.rhsoperand.value == 0
+		end)
+		:maptoarray(
+			function(
+				n: syntax.AstExprBinary
+			): lintTypes.LintViolation -- LUAUFIX: Bidiretional inference of generics should let us not need this annotation
+				return {
+					lintname = name,
+					location = n.location,
+					message = message,
+					severity = "warning",
+					sourcepath = sourcepath,
+				}
+			end
+		)
+end
+
+local rule: lintTypes.LintRule = {
+	name = name,
+	lint = lint,
+}
+
+return table.freeze(rule)