Correct checks for deprecated forecast in weather (#101392)

Co-authored-by: Robert Resch <robert@resch.dev>

homeassistant/components/weather/__init__.py

@@ -8,7 +8,6 @@ from contextlib import suppress
 from dataclasses import dataclass
 from datetime import timedelta
 from functools import partial
-import inspect
 import logging
 from typing import (
     Any,
@@ -56,6 +55,7 @@ from homeassistant.helpers.update_coordinator import (
     DataUpdateCoordinator,
     TimestampDataUpdateCoordinator,
 )
+from homeassistant.loader import async_get_issue_tracker, async_suggest_report_issue
 from homeassistant.util.dt import utcnow
 from homeassistant.util.json import JsonValueType
 from homeassistant.util.unit_system import US_CUSTOMARY_SYSTEM
@@ -296,7 +296,8 @@ class WeatherEntity(Entity, PostInit):
         Literal["daily", "hourly", "twice_daily"],
         list[Callable[[list[JsonValueType] | None], None]],
     ]
-    __weather_legacy_forecast: bool = False
+    __weather_reported_legacy_forecast = False
+    __weather_legacy_forecast = False
 
     _weather_option_temperature_unit: str | None = None
     _weather_option_pressure_unit: str | None = None
@@ -311,15 +312,12 @@ class WeatherEntity(Entity, PostInit):
     def __init_subclass__(cls, **kwargs: Any) -> None:
         """Post initialisation processing."""
         super().__init_subclass__(**kwargs)
-        if any(
+        if (
-            method in cls.__dict__ for method in ("_attr_forecast", "forecast")
+            "forecast" in cls.__dict__
-        ) and not any(
+            and cls.async_forecast_daily is WeatherEntity.async_forecast_daily
-            method in cls.__dict__
+            and cls.async_forecast_hourly is WeatherEntity.async_forecast_hourly
-            for method in (
+            and cls.async_forecast_twice_daily
-                "async_forecast_daily",
+            is WeatherEntity.async_forecast_twice_daily
-                "async_forecast_hourly",
-                "async_forecast_twice_daily",
-            )
         ):
             cls.__weather_legacy_forecast = True
 
@@ -332,38 +330,55 @@ class WeatherEntity(Entity, PostInit):
     ) -> None:
         """Start adding an entity to a platform."""
         super().add_to_platform_start(hass, platform, parallel_updates)
-        _reported_forecast = False
+        if self.__weather_legacy_forecast:
-        if self.__weather_legacy_forecast and not _reported_forecast:
+            self._report_legacy_forecast(hass)
-            module = inspect.getmodule(self)
+
-            if module and module.__file__ and "custom_components" in module.__file__:
+    def _report_legacy_forecast(self, hass: HomeAssistant) -> None:
-                # Do not report on core integrations as they are already fixed or PR is open.
+        """Log warning and create an issue if the entity imlpements legacy forecast."""
-                report_issue = "report it to the custom integration author."
+        if "custom_components" not in type(self).__module__:
-                _LOGGER.warning(
+            # Do not report core integrations as they are already fixed or PR is open.
-                    (
+            return
-                        "%s::%s is using a forecast attribute on an instance of "
+
-                        "WeatherEntity, this is deprecated and will be unsupported "
+        report_issue = async_suggest_report_issue(
-                        "from Home Assistant 2024.3. Please %s"
+            hass,
-                    ),
+            integration_domain=self.platform.platform_name,
-                    self.__module__,
+            module=type(self).__module__,
-                    self.entity_id,
+        )
-                    report_issue,
+        _LOGGER.warning(
-                )
+            (
-                ir.async_create_issue(
+                "%s::%s implements the `forecast` property or sets "
-                    self.hass,
+                "`self._attr_forecast` in a subclass of WeatherEntity, this is "
-                    DOMAIN,
+                "deprecated and will be unsupported from Home Assistant 2024.3."
-                    f"deprecated_weather_forecast_{self.platform.platform_name}",
+                " Please %s"
-                    breaks_in_ha_version="2024.3.0",
+            ),
-                    is_fixable=False,
+            self.platform.platform_name,
-                    is_persistent=False,
+            self.__class__.__name__,
-                    issue_domain=self.platform.platform_name,
+            report_issue,
-                    severity=ir.IssueSeverity.WARNING,
+        )
-                    translation_key="deprecated_weather_forecast",
+
-                    translation_placeholders={
+        translation_placeholders = {"platform": self.platform.platform_name}
-                        "platform": self.platform.platform_name,
+        translation_key = "deprecated_weather_forecast_no_url"
-                        "report_issue": report_issue,
+        issue_tracker = async_get_issue_tracker(
-                    },
+            hass,
-                )
+            integration_domain=self.platform.platform_name,
-                _reported_forecast = True
+            module=type(self).__module__,
+        )
+        if issue_tracker:
+            translation_placeholders["issue_tracker"] = issue_tracker
+            translation_key = "deprecated_weather_forecast_url"
+        ir.async_create_issue(
+            self.hass,
+            DOMAIN,
+            f"deprecated_weather_forecast_{self.platform.platform_name}",
+            breaks_in_ha_version="2024.3.0",
+            is_fixable=False,
+            is_persistent=False,
+            issue_domain=self.platform.platform_name,
+            severity=ir.IssueSeverity.WARNING,
+            translation_key=translation_key,
+            translation_placeholders=translation_placeholders,
+        )
+        self.__weather_reported_legacy_forecast = True
 
     async def async_internal_added_to_hass(self) -> None:
         """Call when the weather entity is added to hass."""
@@ -554,6 +569,15 @@ class WeatherEntity(Entity, PostInit):
 
         Should not be overridden by integrations. Kept for backwards compatibility.
         """
+        if (
+            self._attr_forecast is not None
+            and type(self).async_forecast_daily is WeatherEntity.async_forecast_daily
+            and type(self).async_forecast_hourly is WeatherEntity.async_forecast_hourly
+            and type(self).async_forecast_twice_daily
+            is WeatherEntity.async_forecast_twice_daily
+            and not self.__weather_reported_legacy_forecast
+        ):
+            self._report_legacy_forecast(self.hass)
         return self._attr_forecast
 
     async def async_forecast_daily(self) -> list[Forecast] | None:

homeassistant/components/weather/strings.json

@@ -100,9 +100,13 @@
     }
   },
   "issues": {
-    "deprecated_weather_forecast": {
+    "deprecated_weather_forecast_url": {
-      "title": "The {platform} integration is using deprecated forecast",
+      "title": "The {platform} custom integration is using deprecated weather forecast",
-      "description": "The integration `{platform}` is using the deprecated forecast attribute.\n\nPlease {report_issue}."
+      "description": "The custom integration `{platform}` implements the `forecast` property or sets `self._attr_forecast` in a subclass of WeatherEntity.\n\nPlease create a bug report at {issue_tracker}.\n\nOnce an updated version of `{platform}` is available, install it and restart Home Assistant to fix this issue."
+    },
+    "deprecated_weather_forecast_no_url": {
+      "title": "[%key:component::weather::issues::deprecated_weather_forecast_url::title%]",
+      "description": "The custom integration `{platform}` implements the `forecast` property or sets `self._attr_forecast` in a subclass of WeatherEntity.\n\nPlease report it to the author of the {platform} integration.\n\nOnce an updated version of `{platform}` is available, install it and restart Home Assistant to fix this issue."
     }
   }
 }

tests/components/weather/test_init.py

@@ -1,4 +1,5 @@
 """The test for weather entity."""
+from collections.abc import Generator
 from datetime import datetime
 from typing import Any
 
@@ -8,13 +9,23 @@ from homeassistant.components.weather import (
     ATTR_CONDITION_SUNNY,
     ATTR_FORECAST,
     ATTR_FORECAST_APPARENT_TEMP,
+    ATTR_FORECAST_CLOUD_COVERAGE,
     ATTR_FORECAST_DEW_POINT,
     ATTR_FORECAST_HUMIDITY,
+    ATTR_FORECAST_NATIVE_APPARENT_TEMP,
+    ATTR_FORECAST_NATIVE_DEW_POINT,
+    ATTR_FORECAST_NATIVE_PRECIPITATION,
+    ATTR_FORECAST_NATIVE_PRESSURE,
+    ATTR_FORECAST_NATIVE_TEMP,
+    ATTR_FORECAST_NATIVE_TEMP_LOW,
+    ATTR_FORECAST_NATIVE_WIND_GUST_SPEED,
+    ATTR_FORECAST_NATIVE_WIND_SPEED,
     ATTR_FORECAST_PRECIPITATION,
     ATTR_FORECAST_PRESSURE,
     ATTR_FORECAST_TEMP,
     ATTR_FORECAST_TEMP_LOW,
     ATTR_FORECAST_UV_INDEX,
+    ATTR_FORECAST_WIND_BEARING,
     ATTR_FORECAST_WIND_GUST_SPEED,
     ATTR_FORECAST_WIND_SPEED,
     ATTR_WEATHER_APPARENT_TEMPERATURE,
@@ -44,6 +55,7 @@ from homeassistant.components.weather.const import (
     ATTR_WEATHER_DEW_POINT,
     ATTR_WEATHER_HUMIDITY,
 )
+from homeassistant.config_entries import ConfigEntry, ConfigFlow
 from homeassistant.const import (
     PRECISION_HALVES,
     PRECISION_TENTHS,
@@ -56,6 +68,7 @@ from homeassistant.const import (
 from homeassistant.core import HomeAssistant
 from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import entity_registry as er
+from homeassistant.helpers.entity_platform import AddEntitiesCallback
 import homeassistant.helpers.issue_registry as ir
 from homeassistant.setup import async_setup_component
 from homeassistant.util import dt as dt_util
@@ -69,6 +82,14 @@ from homeassistant.util.unit_system import METRIC_SYSTEM, US_CUSTOMARY_SYSTEM
 
 from . import create_entity
 
+from tests.common import (
+    MockConfigEntry,
+    MockModule,
+    MockPlatform,
+    mock_config_flow,
+    mock_integration,
+    mock_platform,
+)
 from tests.testing_config.custom_components.test import weather as WeatherPlatform
 from tests.testing_config.custom_components.test_weather import (
     weather as NewWeatherPlatform,
@@ -950,7 +971,150 @@ async def test_get_forecast_unsupported(
             )
 
 
-async def test_issue_forecast_deprecated(
+class MockFlow(ConfigFlow):
+    """Test flow."""
+
+
+@pytest.fixture
+def config_flow_fixture(hass: HomeAssistant) -> Generator[None, None, None]:
+    """Mock config flow."""
+    mock_platform(hass, "test.config_flow")
+
+    with mock_config_flow("test", MockFlow):
+        yield
+
+
+ISSUE_TRACKER = "https://blablabla.com"
+
+
+@pytest.mark.parametrize(
+    ("manifest_extra", "translation_key", "translation_placeholders_extra", "report"),
+    [
+        (
+            {},
+            "deprecated_weather_forecast_no_url",
+            {},
+            "report it to the author of the 'test' custom integration",
+        ),
+        (
+            {"issue_tracker": ISSUE_TRACKER},
+            "deprecated_weather_forecast_url",
+            {"issue_tracker": ISSUE_TRACKER},
+            "create a bug report at https://blablabla.com",
+        ),
+    ],
+)
+async def test_issue_forecast_property_deprecated(
+    hass: HomeAssistant,
+    caplog: pytest.LogCaptureFixture,
+    config_flow_fixture: None,
+    manifest_extra: dict[str, str],
+    translation_key: str,
+    translation_placeholders_extra: dict[str, str],
+    report: str,
+) -> None:
+    """Test the issue is raised on deprecated forecast attributes."""
+
+    class MockWeatherMockLegacyForecastOnly(WeatherPlatform.MockWeather):
+        """Mock weather class with mocked legacy forecast."""
+
+        def __init__(self, **values: Any) -> None:
+            """Initialize."""
+            super().__init__(**values)
+            self.forecast_list: list[Forecast] | None = [
+                {
+                    ATTR_FORECAST_NATIVE_TEMP: self.native_temperature,
+                    ATTR_FORECAST_NATIVE_APPARENT_TEMP: self.native_apparent_temperature,
+                    ATTR_FORECAST_NATIVE_TEMP_LOW: self.native_temperature,
+                    ATTR_FORECAST_NATIVE_DEW_POINT: self.native_dew_point,
+                    ATTR_FORECAST_CLOUD_COVERAGE: self.cloud_coverage,
+                    ATTR_FORECAST_NATIVE_PRESSURE: self.native_pressure,
+                    ATTR_FORECAST_NATIVE_WIND_GUST_SPEED: self.native_wind_gust_speed,
+                    ATTR_FORECAST_NATIVE_WIND_SPEED: self.native_wind_speed,
+                    ATTR_FORECAST_WIND_BEARING: self.wind_bearing,
+                    ATTR_FORECAST_UV_INDEX: self.uv_index,
+                    ATTR_FORECAST_NATIVE_PRECIPITATION: self._values.get(
+                        "native_precipitation"
+                    ),
+                    ATTR_FORECAST_HUMIDITY: self.humidity,
+                }
+            ]
+
+        @property
+        def forecast(self) -> list[Forecast] | None:
+            """Return the forecast."""
+            return self.forecast_list
+
+    # Fake that the class belongs to a custom integration
+    MockWeatherMockLegacyForecastOnly.__module__ = "custom_components.test.weather"
+
+    kwargs = {
+        "native_temperature": 38,
+        "native_temperature_unit": UnitOfTemperature.CELSIUS,
+    }
+    weather_entity = MockWeatherMockLegacyForecastOnly(
+        name="Testing",
+        entity_id="weather.testing",
+        condition=ATTR_CONDITION_SUNNY,
+        **kwargs,
+    )
+
+    async def async_setup_entry_init(
+        hass: HomeAssistant, config_entry: ConfigEntry
+    ) -> bool:
+        """Set up test config entry."""
+        await hass.config_entries.async_forward_entry_setups(config_entry, [DOMAIN])
+        return True
+
+    async def async_setup_entry_weather_platform(
+        hass: HomeAssistant,
+        config_entry: ConfigEntry,
+        async_add_entities: AddEntitiesCallback,
+    ) -> None:
+        """Set up test weather platform via config entry."""
+        async_add_entities([weather_entity])
+
+    mock_integration(
+        hass,
+        MockModule(
+            "test",
+            async_setup_entry=async_setup_entry_init,
+            partial_manifest=manifest_extra,
+        ),
+        built_in=False,
+    )
+    mock_platform(
+        hass,
+        "test.weather",
+        MockPlatform(async_setup_entry=async_setup_entry_weather_platform),
+    )
+
+    config_entry = MockConfigEntry(domain="test")
+    config_entry.add_to_hass(hass)
+    assert await hass.config_entries.async_setup(config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert weather_entity.state == ATTR_CONDITION_SUNNY
+
+    issues = ir.async_get(hass)
+    issue = issues.async_get_issue("weather", "deprecated_weather_forecast_test")
+    assert issue
+    assert issue.issue_domain == "test"
+    assert issue.issue_id == "deprecated_weather_forecast_test"
+    assert issue.translation_key == translation_key
+    assert (
+        issue.translation_placeholders
+        == {"platform": "test"} | translation_placeholders_extra
+    )
+
+    assert (
+        "test::MockWeatherMockLegacyForecastOnly implements the `forecast` property or "
+        "sets `self._attr_forecast` in a subclass of WeatherEntity, this is deprecated "
+        f"and will be unsupported from Home Assistant 2024.3. Please {report}"
+    ) in caplog.text
+
+
+async def test_issue_forecast_attr_deprecated(
     hass: HomeAssistant,
     enable_custom_integrations: None,
     caplog: pytest.LogCaptureFixture,
@@ -964,14 +1128,14 @@ async def test_issue_forecast_deprecated(
     platform: WeatherPlatform = getattr(hass.components, "test.weather")
     caplog.clear()
     platform.init(empty=True)
-    platform.ENTITIES.append(
+    weather = platform.MockWeather(
-        platform.MockWeatherMockLegacyForecastOnly(
+        name="Testing",
-            name="Testing",
+        entity_id="weather.testing",
-            entity_id="weather.testing",
+        condition=ATTR_CONDITION_SUNNY,
-            condition=ATTR_CONDITION_SUNNY,
+        **kwargs,
-            **kwargs,
-        )
     )
+    weather._attr_forecast = []
+    platform.ENTITIES.append(weather)
 
     entity0 = platform.ENTITIES[0]
     assert await async_setup_component(
@@ -986,15 +1150,15 @@ async def test_issue_forecast_deprecated(
     assert issue
     assert issue.issue_domain == "test"
     assert issue.issue_id == "deprecated_weather_forecast_test"
-    assert issue.translation_placeholders == {
+    assert issue.translation_key == "deprecated_weather_forecast_no_url"
-        "platform": "test",
+    assert issue.translation_placeholders == {"platform": "test"}
-        "report_issue": "report it to the custom integration author.",
-    }
 
     assert (
-        "custom_components.test.weather::weather.testing is using a forecast attribute on an instance of WeatherEntity"
+        "test::MockWeather implements the `forecast` property or "
-        in caplog.text
+        "sets `self._attr_forecast` in a subclass of WeatherEntity, this is deprecated "
-    )
+        "and will be unsupported from Home Assistant 2024.3. Please report it to the "
+        "author of the 'test' custom integration"
+    ) in caplog.text
 
 
 async def test_issue_forecast_deprecated_no_logging(

tests/testing_config/custom_components/test/weather.py

@@ -125,11 +125,6 @@ class MockWeather(MockEntity, WeatherEntity):
         """Return the unit of measurement for visibility."""
         return self._handle("native_visibility_unit")
 
-    @property
-    def forecast(self) -> list[Forecast] | None:
-        """Return the forecast."""
-        return self._handle("forecast")
-
     @property
     def native_precipitation_unit(self) -> str | None:
         """Return the native unit of measurement for accumulated precipitation."""
@@ -291,34 +286,3 @@ class MockWeatherMockForecast(MockWeather):
                 ATTR_FORECAST_HUMIDITY: self.humidity,
             }
         ]
-
-
-class MockWeatherMockLegacyForecastOnly(MockWeather):
-    """Mock weather class with mocked legacy forecast."""
-
-    def __init__(self, **values: Any) -> None:
-        """Initialize."""
-        super().__init__(**values)
-        self.forecast_list: list[Forecast] | None = [
-            {
-                ATTR_FORECAST_NATIVE_TEMP: self.native_temperature,
-                ATTR_FORECAST_NATIVE_APPARENT_TEMP: self.native_apparent_temperature,
-                ATTR_FORECAST_NATIVE_TEMP_LOW: self.native_temperature,
-                ATTR_FORECAST_NATIVE_DEW_POINT: self.native_dew_point,
-                ATTR_FORECAST_CLOUD_COVERAGE: self.cloud_coverage,
-                ATTR_FORECAST_NATIVE_PRESSURE: self.native_pressure,
-                ATTR_FORECAST_NATIVE_WIND_GUST_SPEED: self.native_wind_gust_speed,
-                ATTR_FORECAST_NATIVE_WIND_SPEED: self.native_wind_speed,
-                ATTR_FORECAST_WIND_BEARING: self.wind_bearing,
-                ATTR_FORECAST_UV_INDEX: self.uv_index,
-                ATTR_FORECAST_NATIVE_PRECIPITATION: self._values.get(
-                    "native_precipitation"
-                ),
-                ATTR_FORECAST_HUMIDITY: self.humidity,
-            }
-        ]
-
-    @property
-    def forecast(self) -> list[Forecast] | None:
-        """Return the forecast."""
-        return self.forecast_list