overhault rule/control internals

instead of keeping them as flat variables, prefix all internals with `__` to create consistency. Also add accessors on the class-level to expose these values in all rules. This way we keep all variable-names in one location and get some safety on access.

lib/inspec/dsl.rb

@@ -35,7 +35,7 @@ module Inspec::DSL
   # let it run. This happens after everything
   # else is merged in.
   def self.execute_rule(r, profile_id)
-    checks = r.instance_variable_get(:@checks)
+    checks = ::Inspec::Rule.checks(r)
     fid = InspecBaseRule.full_id(r, profile_id)
     checks.each do |m, a, b|
       # check if the resource is skippable and skipped

lib/inspec/profile.rb

@@ -256,7 +256,7 @@ module Inspec
           impact: rule.impact,
           refs: rule.ref,
           tags: rule.tag,
-          checks: rule.instance_variable_get(:@checks),
+          checks: Inspec::Rule.checks(rule),
           code: rule.instance_variable_get(:@__code),
           source_location: rule.instance_variable_get(:@__source_location),
           group_title: rule.instance_variable_get(:@__group_title),

lib/inspec/profile_context.rb

@@ -136,7 +136,7 @@ module Inspec
 
           # Skip the control if the resource triggered a skip;
           if @skip_profile
-            control.instance_variable_set(:@checks, [])
+            ::Inspec::Rule.set_checks(control, [])
             # TODO: we use os as the carrier here, but should consider
             # a separate resource to do skipping
             resource = os

lib/inspec/rule.rb

@@ -15,16 +15,18 @@ module Inspec
     def initialize(id, _opts, &block)
       @id = id
       @impact = nil
-      @__block = block
-      @__code = __get_block_source(&block)
-      @__source_location = __get_block_source_location(&block)
       @title = nil
       @desc = nil
       @refs = []
       @tags = {}
+
       # not changeable by the user:
-      @profile_id = nil
-      @checks = []
+      @__block = block
+      @__code = __get_block_source(&block)
+      @__source_location = __get_block_source_location(&block)
+      @__rule_id = nil
+      @__checks = []
+
       # evaluate the given definition
       instance_eval(&block) if block_given?
     end
@@ -87,25 +89,39 @@ module Inspec
         dsl = self.class.ancestors[1]
         Class.new(DescribeBase) do
           include dsl
-        end.new(method(:add_check))
+        end.new(method(:__add_check))
       else
-        add_check('describe', values, block)
+        __add_check('describe', values, block)
       end
     end
 
     def expect(value, &block)
       target = Inspec::Expect.new(value, &block)
-      add_check('expect', [value], target)
+      __add_check('expect', [value], target)
       target
     end
 
+    def self.rule_id(rule)
+      rule.instance_variable_get(:@__rule_id)
+    end
+
+    def self.set_rule_id(rule, value)
+      rule.instance_variable_set(:@__rule_id, value)
+    end
+
+    def self.checks(rule)
+      rule.instance_variable_get(:@__checks)
+    end
+
+    end
+
     def self.merge(dst, src)
       if src.id != dst.id
         # TODO: register an error, this case should not happen
         return
       end
-      sp = src.instance_variable_get(:@profile_id)
-      dp = dst.instance_variable_get(:@profile_id)
+      sp = rule_id(src)
+      dp = rule_id(dst)
       if sp != dp
         # TODO: register an error, this case should not happen
         return
@@ -117,10 +133,8 @@ module Inspec
       # merge indirect fields
       # checks defined in the source will completely eliminate
       # all checks that were defined in the destination
-      sc = src.instance_variable_get(:@checks)
-      unless sc.nil? || sc.empty?
-        dst.instance_variable_set(:@checks, sc)
-      end
+      sc = checks(src)
+      dst.instance_variable_set(:@__checks, sc) unless sc.empty?
     end
 
     # Get the full id consisting of profile id + rule id
@@ -140,11 +154,8 @@ module Inspec
           return nil
         end
       end
-      pid = rule.instance_variable_get(:@profile_id)
-      if pid.nil?
-        rule.instance_variable_set(:@profile_id, profile_id)
-        pid = profile_id
-      end
+      pid = rule_id(rule)
+      pid = set_rule_id(rule, profile_id) if pid.nil?
 
       # if we don't have a profile id, just return the rule's ID
       return rid if pid.nil? or pid.empty?
@@ -154,8 +165,8 @@ module Inspec
 
     private
 
-    def add_check(describe_or_expect, values, block)
-      @checks.push([describe_or_expect, values, block])
+    def __add_check(describe_or_expect, values, block)
+      @__checks.push([describe_or_expect, values, block])
     end
 
     # Idio(ma)tic unindent

lib/inspec/runner.rb

@@ -154,7 +154,7 @@ module Inspec
 
     def register_rule(rule_id, rule)
       @rules[rule_id] = rule
-      checks = rule.instance_variable_get(:@checks)
+      checks = ::Inspec::Rule.checks(rule)
       examples = checks.map do |m, a, b|
         get_check_example(m, a, b)
       end.flatten.compact

test/unit/profile_context_test.rb

@@ -60,7 +60,7 @@ describe Inspec::ProfileContext do
   end
 
   def get_checks
-    get_rule.instance_variable_get(:@checks)
+    Inspec::Rule.checks(get_rule)
   end
 
   it 'must be able to load empty content' do
@@ -189,7 +189,7 @@ describe Inspec::ProfileContext do
     it 'doesnt add any checks if none are provided' do
       profile.load("rule #{rule_id.inspect}")
       rule = profile.rules[rule_id]
-      rule.instance_variable_get(:@checks).must_equal([])
+      Inspec::Rule.checks(rule).must_equal([])
     end
 
     describe 'supports empty describe blocks' do