Repository: useblocks/sphinx-needs
Pull request #1310 (2024-10-02 11:40:16)

🔧 Make direct access to `env` attributes private
These attributes should always be accessed via the API interface,
adding `_` to them makes this contract more explicit.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1310?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 87.74%. Comparing base [(`4e10030`)](https://app.codecov.io/gh/useblocks/sphinx-needs/commit/4e10030581c03c59764e517ab750d3ddbb2695d8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) to head [(`1fa5bb8`)](https://app.codecov.io/gh/useblocks/sphinx-needs/commit/1fa5bb8233eac1ab7f424ec6c81dfdbaa9653ca7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks).
> Report is 58 commits behind head on master.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #1310      +/-   ##
==========================================
+ Coverage   86.87%   87.74%   +0.86%     
==========================================
  Files          56       60       +4     
  Lines        6532     7032     +500     
==========================================
+ Hits         5675     6170     +495     
- Misses        857      862       +5     
```

| [Flag](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1310/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) | Coverage Δ | |
|---|---|---|
| [pytests](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks) | `87.74% <100.00%> (+0.86%)` | :arrow_up: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/useblocks/sphinx-needs/pull/1310?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=useblocks).


Patch:
diff --git a/sphinx_needs/data.py b/sphinx_needs/data.py
index b6b62b55..aa9448a4 100644
--- a/sphinx_needs/data.py
+++ b/sphinx_needs/data.py
@@ -764,23 +764,23 @@ class SphinxNeedsData:
         This is lazily created and cached in the environment.
         """
         try:
-            return self.env.needs_all_docs
+            return self.env._needs_all_docs
         except AttributeError:
-            self.env.needs_all_docs = {"all": []}
-        return self.env.needs_all_docs
+            self.env._needs_all_docs = {"all": []}
+        return self.env._needs_all_docs
 
     @property
     def needs_is_post_processed(self) -> bool:
         """Whether needs have been post-processed."""
         try:
-            return self.env.needs_is_post_processed
+            return self.env._needs_is_post_processed
         except AttributeError:
-            self.env.needs_is_post_processed = False
-        return self.env.needs_is_post_processed
+            self.env._needs_is_post_processed = False
+        return self.env._needs_is_post_processed
 
     @needs_is_post_processed.setter
     def needs_is_post_processed(self, value: bool) -> None:
-        self.env.needs_is_post_processed = value
+        self.env._needs_is_post_processed = value
 
     def get_or_create_services(self) -> ServiceManager:
         """Get information about services.
@@ -790,10 +790,10 @@ class SphinxNeedsData:
         from sphinx_needs.services.manager import ServiceManager
 
         try:
-            return self.env.app.needs_services
+            return self.env.app._needs_services
         except AttributeError:
-            self.env.app.needs_services = ServiceManager(self.env.app)
-        return self.env.app.needs_services
+            self.env.app._needs_services = ServiceManager(self.env.app)
+        return self.env.app._needs_services
 
     def get_or_create_extends(self) -> dict[str, NeedsExtendType]:
         """Get all need modifications, mapped by ID.
@@ -801,10 +801,10 @@ class SphinxNeedsData:
         This is lazily created and cached in the environment.
         """
         try:
-            return self.env.need_all_needextend
+            return self.env._need_all_needextend
         except AttributeError:
-            self.env.need_all_needextend = {}
-        return self.env.need_all_needextend
+            self.env._need_all_needextend = {}
+        return self.env._need_all_needextend
 
     def get_or_create_umls(self) -> dict[str, NeedsUmlType]:
         """Get all need uml diagrams, mapped by ID.
@@ -812,18 +812,18 @@ class SphinxNeedsData:
         This is lazily created and cached in the environment.
         """
         try:
-            return self.env.needs_all_needumls
+            return self.env._needs_all_needumls
         except AttributeError:
-            self.env.needs_all_needumls = {}
-        return self.env.needs_all_needumls
+            self.env._needs_all_needumls = {}
+        return self.env._needs_all_needumls
 
     @property
     def _needs_all_nodes(self) -> dict[str, Need]:
         try:
-            return self.env.needs_all_nodes
+            return self.env._needs_all_nodes
         except AttributeError:
-            self.env.needs_all_nodes = {}
-        return self.env.needs_all_nodes
+            self.env._needs_all_nodes = {}
+        return self.env._needs_all_nodes
 
     def set_need_node(self, need_id: str, node: Need) -> None:
         """Set a need node in the cache."""
@@ -906,7 +906,7 @@ def merge_data(
                     f"not {type(other_objects)} and {type(objects)}"
                 )
 
-    _merge("needs_all_docs", is_complex_dict=True)
-    _merge("needs_all_nodes")
-    _merge("need_all_needextend")
-    _merge("needs_all_needumls")
+    _merge("_needs_all_docs", is_complex_dict=True)
+    _merge("_needs_all_nodes")
+    _merge("_need_all_needextend")
+    _merge("_needs_all_needumls")
diff --git a/sphinx_needs/needs.py b/sphinx_needs/needs.py
index 74211449..34ce5913 100644
--- a/sphinx_needs/needs.py
+++ b/sphinx_needs/needs.py
@@ -499,10 +499,9 @@ def prepare_env(app: Sphinx, env: BuildEnvironment, _docname: str) -> None:
     """
     needs_config = NeedsSphinxConfig(app.config)
     data = SphinxNeedsData(env)
-    data.get_or_create_docs()
-    services = data.get_or_create_services()
 
     # Register embedded services
+    services = data.get_or_create_services()
     services.register("github-issues", GithubService, gh_type="issue")
     services.register("github-prs", GithubService, gh_type="pr")
     services.register("github-commits", GithubService, gh_type="commit")
diff --git a/sphinx_needs/warnings.py b/sphinx_needs/warnings.py
index 796c727f..5c0e7860 100644
--- a/sphinx_needs/warnings.py
+++ b/sphinx_needs/warnings.py
@@ -45,10 +45,10 @@ def process_warnings(app: Sphinx, exception: Exception | None) -> None:
     # Check if warnings already got executed.
     # Needed because the used event gets executed multiple times, but warnings need to be checked only
     # on first execution
-    if hasattr(env, "needs_warnings_executed") and env.needs_warnings_executed:
+    if hasattr(env, "_needs_warnings_executed") and env._needs_warnings_executed:
         return
 
-    env.needs_warnings_executed = True  # type: ignore[attr-defined]
+    env._needs_warnings_executed = True  # type: ignore[attr-defined]
 
     # Exclude external needs for warnings check
     needs_view = needs_view.filter_is_external(False)


Test patch:
diff --git a/tests/test_needarch.py b/tests/test_needarch.py
index 0d68df36..a6a8467d 100644
--- a/tests/test_needarch.py
+++ b/tests/test_needarch.py
@@ -51,7 +51,7 @@ def test_doc_needarch_jinja_import(test_app, snapshot):
     assert html
 
     # check needarch
-    all_needumls = app.env.needs_all_needumls
+    all_needumls = app.env._needs_all_needumls
     assert all_needumls == snapshot
 
 
@@ -64,7 +64,7 @@ def test_needarch_jinja_func_need(test_app, snapshot):
     app = test_app
     app.build()
 
-    all_needumls = app.env.needs_all_needumls
+    all_needumls = app.env._needs_all_needumls
     assert all_needumls == snapshot
 
     html = Path(app.outdir, "index.html").read_text(encoding="utf8")
diff --git a/tests/test_needuml.py b/tests/test_needuml.py
index 41f166fd..2e8ca0c0 100644
--- a/tests/test_needuml.py
+++ b/tests/test_needuml.py
@@ -20,7 +20,7 @@ def test_doc_build_html(test_app, snapshot):
     all_needs = dict(SphinxNeedsData(app.env).get_needs_view())
     assert all_needs == snapshot()
 
-    all_needumls = app.env.needs_all_needumls
+    all_needumls = app.env._needs_all_needumls
     assert all_needumls == snapshot
 
 
@@ -169,7 +169,7 @@ def test_needuml_filter(test_app, snapshot):
     app = test_app
     app.build()
 
-    all_needumls = app.env.needs_all_needumls
+    all_needumls = app.env._needs_all_needumls
     assert all_needumls == snapshot
 
     html = Path(app.outdir, "index.html").read_text(encoding="utf8")
@@ -193,7 +193,7 @@ def test_needuml_jinja_func_flow(test_app, snapshot):
     app = test_app
     app.build()
 
-    all_needumls = app.env.needs_all_needumls
+    all_needumls = app.env._needs_all_needumls
     assert all_needumls == snapshot
 
     html = Path(app.outdir, "index.html").read_text(encoding="utf8")
@@ -267,7 +267,7 @@ def test_needuml_jinja_func_ref(test_app, snapshot):
     app = test_app
     app.build()
 
-    all_needumls = app.env.needs_all_needumls
+    all_needumls = app.env._needs_all_needumls
     assert all_needumls == snapshot
 
     html = Path(app.outdir, "index.html").read_text(encoding="utf8")
diff --git a/tests/test_open_needs_service.py b/tests/test_open_needs_service.py
index 0f0592e2..211eff3e 100644
--- a/tests/test_open_needs_service.py
+++ b/tests/test_open_needs_service.py
@@ -91,9 +91,9 @@ def test_ons_service(test_app):
     )
 
     test_app.build()
-    assert isinstance(test_app.needs_services, ServiceManager)
+    assert isinstance(test_app._needs_services, ServiceManager)
 
-    manager = test_app.needs_services
+    manager = test_app._needs_services
     service = manager.get("open-needs")
     assert hasattr(service, "content")
 
diff --git a/tests/test_services/test_service_basics.py b/tests/test_services/test_service_basics.py
index f0b3254f..89e8e052 100644
--- a/tests/test_services/test_service_basics.py
+++ b/tests/test_services/test_service_basics.py
@@ -61,9 +61,9 @@ class NoDebugService(BaseService):
 def test_service_creation(test_app):
     app = test_app
     app.build()
-    assert isinstance(app.needs_services, ServiceManager)
+    assert isinstance(app._needs_services, ServiceManager)
 
-    manager = app.needs_services
+    manager = app._needs_services
     service = manager.get("testservice")
     assert hasattr(service, "custom_option")