Add QGS107 and improve documentation [GispoCoding#8]

README.md

@@ -33,8 +33,9 @@ Just call `flake8 .` in your package or `flake your.py`.
 * `QGS102`: Don't use imports from qgis protected members ([example](#QGS102))
 * `QGS103`: Don't use from-imports from PyQt directly ([example](#QGS103))
 * `QGS104`: Don't use imports from PyQt directly ([example](#QGS104))
-* `QGS105`: Don't pass QgisInterface as an argument ([example](#QGS105))
+* `QGS105`: Don't pass QgisInterface as an argument ([example](#QGS105)). If you want to use this rule, disable rule `QGS107`.
 * `QGS106`: Don't import gdal directly, import if from osgeo package ([example](#QGS106))
+* `QGS107`: Don't import QgisInterface instance (iface). Instead pass it as an argument ([example](#QGS107)). If you want to use this rule, disable rule `QGS105`.
 
 
 
@@ -43,9 +44,14 @@ To do that, use the standard Flake8 configuration. For example, within the `setu
 
 ```python
 [flake8]
-ignore = QGS101, QGS102
+ignore = QGS101, QGS107
 ```
 
+## Rule exclusive choices
+For QgisInterface, activate only `QGS105` OR `QGS107`, do not enable both as they cancel each other's out. If you want to use both approaches mixed, simply ignore both rules.
+Both ways of getting iface are valid, but there might be valid reason to chooce which one to use.
+* `QGS105`: it is much easier to import QgisInterface and it's easier to [mock](https://github.com/GispoCoding/pytest-qgis#hooks) it as well when writing tests. This approach is not however documented properly, so the API might change at some point to exclude this.
+* `QGS107`: this is the documented way of getting QgisInterface in plugins. However it requires writing more code.
 
 ## Examples
 

flake8_qgis/flake8_qgis.py

@@ -36,9 +36,13 @@
 IMPORT_USE_INSTEAD_OF = "{code} Use 'import {correct}' instead of 'import {incorrect}'"
 QGS105 = (
     "QGS105 Do not pass iface (QgisInterface) as an argument, "
-    "instead import it: 'from qgis.utils import iface'"
+    "instead import it: 'from qgis.utils import iface' or use the rule QGS107"
 )
 QGS106 = "QGS106 Use 'from osgeo import {members}' instead of 'import {members}'"
+QGS107 = (
+    "QGS107 Do not import iface (QgisInterface), instead "
+    "pass it as an argument or use the rule QGS105"
+)
 
 
 def _test_qgis_module(module: Optional[str]) -> Optional[str]:
@@ -138,13 +142,21 @@ def _get_qgs106(node: ast.Import) -> List["FlakeError"]:
     return errors
 
 
+def _get_qgs107(node: ast.ImportFrom) -> List["FlakeError"]:
+    errors: List["FlakeError"] = []
+    if node.module == "qgis.utils" and "iface" in [name.name for name in node.names]:
+        errors.append((node.lineno, node.col_offset, QGS107))
+    return errors
+
+
 class Visitor(ast.NodeVisitor):
     def __init__(self) -> None:
         self.errors: List["FlakeError"] = []
 
     def visit_ImportFrom(self, node: ast.ImportFrom) -> None:  # noqa: N802
         self.errors += _test_module_at_import_from("QGS101", node, _test_qgis_module)
         self.errors += _test_module_at_import_from("QGS103", node, _test_pyqt_module)
+        self.errors += _get_qgs107(node)
         self.generic_visit(node)
 
     def visit_Import(self, node: Import) -> None:  # noqa: N802

tests/test_flake8_qgis.py

@@ -1,16 +1,16 @@
 # Core Library
 # Core Library modules
 import ast
-from typing import Set
 
 # First party modules
 # First party
 from flake8_qgis import Plugin
+from flake8_qgis.flake8_qgis import QGS105, QGS107
 
 """Tests for `flake8_qgis` package."""
 
 
-def _results(s: str) -> Set[str]:
+def _results(s: str) -> set[str]:
     tree = ast.parse(s)
     plugin = Plugin(tree)
     return {f"{line}:{col} {msg}" for line, col, msg, _ in plugin.run()}
@@ -110,10 +110,7 @@ def some_function(somearg, iface):
     pass
         """
     )
-    assert ret == {
-        "2:0 QGS105 Do not pass iface (QgisInterface) as an argument, instead import "
-        "it: 'from qgis.utils import iface'"
-    }
+    assert ret == {"2:0 " + QGS105}
 
 
 def test_QGS105_typed():
@@ -123,10 +120,7 @@ def some_function(somearg, interface: QgisInterface):
     pass
         """
     )
-    assert ret == {
-        "2:0 QGS105 Do not pass iface (QgisInterface) as an argument, instead import "
-        "it: 'from qgis.utils import iface'"
-    }
+    assert ret == {"2:0 " + QGS105}
 
 
 def test_QGS105_method():
@@ -137,10 +131,7 @@ def some_method(self, somearg, iface):
         pass
         """
     )
-    assert ret == {
-        "3:4 QGS105 Do not pass iface (QgisInterface) as an argument, instead import "
-        "it: 'from qgis.utils import iface'"
-    }
+    assert ret == {"3:4 " + QGS105}
 
 
 def test_QGS105_static_method():
@@ -153,10 +144,7 @@ def some_method(somearg, iface):
         """
     )
     assert len(ret) == 1
-    assert list(ret)[0].endswith(
-        "QGS105 Do not pass iface (QgisInterface) as an argument, instead import "
-        "it: 'from qgis.utils import iface'"
-    )
+    assert list(ret)[0].endswith(QGS105)
 
 
 def test_QGS106_pass():
@@ -172,3 +160,8 @@ def test_QGS106():
         "1:0 QGS106 Use 'from osgeo import gdal' instead of 'import gdal'",
         "1:0 QGS106 Use 'from osgeo import ogr' instead of 'import ogr'",
     }
+
+
+def test_QGS107():
+    ret = _results("from qgis.utils import iface")
+    assert ret == {"1:0 " + QGS107}