feat: warn on mutated loop control variables (#109)

Author: arichard4

docsrc/cli.rst

@@ -120,6 +120,7 @@ Option                                  Meaning
 ``--enable | -e <patt> [<patt>] ...``   Do not filter out warnings matching patterns.
 ``--only | -o <patt> [<patt>] ...``     Filter out warnings not matching patterns.
 ``--operators <patt> [<patt>] ...``     Allow compound operators matching patterns. (Multiple assignment not supported, as this is specifically for the Playdate SDK.)
+``--const-loop-control-vars``           Flag loop control variables as <const>.
 ``--config <config>``                   Path to custom configuration file (default: ``.luacheckrc``).
 ``--no-config``                         Do not look up custom configuration file.
 ``--default-config <config>``           Default path to custom configuration file, to be used if ``--[no-]config`` is not used and ``.luacheckrc`` is not found.

docsrc/config.rst

@@ -43,6 +43,7 @@ Option                        Type                                     Default v
 ``new_read_globals``          Array of strings or field definition map (Do not overwrite)
 ``not_globals``               Array of strings                         ``{}``
 ``operators``                 Array of strings                         ``{}``
+``const_loop_control_vars``   Boolean                                  ``false``
 ``compat``                    Boolean                                  ``false``
 ``allow_defined``             Boolean                                  ``false``
 ``allow_defined_top``         Boolean                                  ``false``

docsrc/warnings.rst

@@ -45,6 +45,7 @@ Code Description
 432  Shadowing an upvalue argument.
 433  Shadowing an upvalue loop variable.
 441  Constant local variable is modified.
+442  Loop control variable is modified.
 511  Unreachable code.
 512  Loop can be executed at most once.
 521  Unused label.

spec/linearize_spec.lua

@@ -476,6 +476,25 @@ end
 
 a, b = 3, 3
 print(a, b)
+]]))
+   end)
+
+   it("handles loop control variables correctly", function()
+      assert.same({
+         {code = "442", line = 2, column = 3, end_column = 11, name = 'i',
+            defined_line = 1},
+         {code = "442", line = 7, column = 3, end_column = 13, name = 'k',
+            defined_line = 6},
+      }, helper.get_stage_warnings("linearize", [[
+for i = 1, 10 do
+  i = i - 1
+  print(i)
+end
+
+for k,_ in pairs({ foo = "bar" }) do
+  k = "k:"..k
+  print(k)
+end
 ]]))
    end)
 end)

spec/parser_spec.lua

@@ -242,7 +242,7 @@ describe("parser", function()
    describe("when parsing for", function()
       it("parses fornum correctly", function()
          assert.same({tag = "Fornum",
-                        {tag = "Id", "i"},
+                        {tag = "Id", const_loop_var = true, "i"},
                         {tag = "Number", "1"},
                         {tag = "Op", "len", {tag = "Id", "t"}},
                         {}
@@ -283,7 +283,7 @@ describe("parser", function()
 
       it("parses fornum with step correctly", function()
          assert.same({tag = "Fornum",
-                        {tag = "Id", "i"},
+                        {tag = "Id", const_loop_var = true, "i"},
                         {tag = "Number", "1"},
                         {tag = "Op", "len", {tag = "Id", "t"}},
                         {tag = "Number", "2"},
@@ -297,14 +297,14 @@ describe("parser", function()
 
       it("parses forin correctly", function()
          assert.same({tag = "Forin", {
-                           {tag = "Id", "i"}
+                           {tag = "Id", const_loop_var = true, "i"}
                         }, {
                            {tag = "Id", "t"}
                         },
                         {}
                      }, get_node("for i in t do end"))
          assert.same({tag = "Forin", {
-                           {tag = "Id", "i"},
+                           {tag = "Id", const_loop_var = true, "i"},
                            {tag = "Id", "j"}
                         }, {
                            {tag = "Id", "t"},
@@ -987,7 +987,7 @@ describe("parser", function()
             },
             {tag = "Do",
                {tag = "Fornum",
-                  {tag = "Id", "i"},
+                  {tag = "Id", const_loop_var = true, "i"},
                   {tag = "Number", "1"},
                   {tag = "Number", "2"},
                   {
@@ -998,7 +998,7 @@ describe("parser", function()
                },
                {tag = "Forin",
                   {
-                     {tag = "Id", "k"},
+                     {tag = "Id", const_loop_var = true, "k"},
                      {tag = "Id", "v"}
                   },
                   {

src/luacheck/parser.lua

@@ -719,14 +719,15 @@ statements["for"] = function(state)
 
    local ast_node = {}
    local tag
-   local first_var = parse_id(state)
+   local control_var = parse_id(state)
+   control_var.const_loop_var = true
 
    if state.token == "=" then
       -- Numeric "for" loop.
       tag = "Fornum"
       -- Skip "=".
       skip_token(state)
-      ast_node[1] = first_var
+      ast_node[1] = control_var
       ast_node[2] = parse_expression(state)
       check_and_skip_token(state, ",")
       ast_node[3] = parse_expression(state)
@@ -741,7 +742,7 @@ statements["for"] = function(state)
       -- Generic "for" loop.
       tag = "Forin"
 
-      local iter_vars = {first_var}
+      local iter_vars = {control_var}
       while test_and_skip_token(state, ",") do
          iter_vars[#iter_vars + 1] = parse_id(state)
       end

src/luacheck/stages/linearize.lua

@@ -24,6 +24,11 @@ stage.warnings = {
       message_format = "variable {name!} was defined as const on line {defined_line}",
       fields = {"name", "defined_line"}
    },
+   ["442"] = {
+      message_format = 
+         "variable {name!}, defined on line {defined_line}, is a loop control variable and shouldn't be modified",
+      fields = {"name", "defined_line"}
+   },
    ["521"] = {message_format = "unused label {label!}", fields = {"label"}}
 }
 
@@ -47,7 +52,8 @@ local function warn_redefined(chstate, var, prev_var, is_same_scope)
 end
 
 local function warn_modified_const_label(chstate, node, var)
-   chstate:warn_range("441", node, {
+   local code = "44" .. (var.node.const and "1" or "2")
+   chstate:warn_range(code, node, {
       defined_line = var.node.line,
       name = var.name
    })
@@ -533,6 +539,10 @@ function LinState:emit_stmt_Set(node)
             if var.node.const then
                warn_modified_const_label(self.chstate, node, var)
             end
+
+            if var.node.const_loop_var then
+               warn_modified_const_label(self.chstate, node, var)
+            end
          end
       else
          assert(expr.tag == "Index")

src/luacheck/standards.lua

@@ -128,7 +128,8 @@ local function add_fields(def, fields, overwrite, ignore_array_part, default_rea
       return
    end
 
-   for field_name, field_def in pairs(fields) do
+   for k, v in pairs(fields) do
+      local field_name, field_def = k, v
       if type(field_name) == "string" or not ignore_array_part then
          if type(field_name) ~= "string" then
             field_name = field_def