/**
 * @name Error-prone name of loop variable
 * @description The iteration variable of a nested loop should have a descriptive name: short names like i, j, or k can cause confusion except in very simple loops.
 * @kind problem
 * @id cpp/short-loop-var-name
 * @problem.severity recommendation
 * @precision medium
 * @tags maintainability
 *       readability
 */

import cpp

predicate short(Variable v) { v.getName().length() = 1 }

predicate forStmtAncestor(Stmt child, ForStmt parent) {
  child.getParent() = parent or forStmtAncestor(child.getParent(), parent)
}

/**
 * Gets an `ArrayExpr` that's nested directly inside `ArrayExpr ae`.
 */
ArrayExpr getANestedArrayExpr(ArrayExpr ae) { result.getArrayBase() = ae }

/**
 * Holds if variables `a` and `b` are accessed in a way that looks like they
 * are a coordinate pair.  For example:
 * ```
 *  arr[x][y]
 *  arr[(y * width) + x]
 * ```
 */
predicate coordinatePair(Variable a, Variable b) {
  exists(ArrayExpr ae |
    getANestedArrayExpr*(ae).getArrayOffset().getAChild*() = a.getAnAccess() and
    getANestedArrayExpr*(ae).getArrayOffset().getAChild*() = b.getAnAccess() and
    not a = b
  )
}

from ForStmt outer, ForStmt inner, Variable iterationVar, Variable innerVar
where
  forStmtAncestor(inner, outer) and
  short(innerVar) and
  iterationVar = outer.getAnIterationVariable() and
  innerVar = inner.getAnIterationVariable() and
  short(iterationVar) and
  not coordinatePair(iterationVar, innerVar)
select iterationVar,
  "Iteration variable " + iterationVar.getName() +
    " for $@ should have a descriptive name, since there is a $@.", outer, "this loop", inner,
  "nested loop"