Dev - #7
Merged
Merged
Conversation
…Flet
O gerador de análise emitia o widget como `{Control}Widget` em `{snake}_widget.dart` mais um `lib/extension.dart` legado com `createControl`, enquanto o skeleton (e o template oficial do Flet) exportam `lib/src/extension.dart` com `class Extension extends FletExtension`, que importa `{snake}_control.dart` e referencia `{Control}Control`. Esse mismatch fazia o entry-point exportado não compilar (`uri_does_not_exist` + `undefined_method` para `{Control}Control`) em toda extensão ui_control.
- dart_service.py: widget class/arquivo agora `{Control}Control` / `{snake}_control.dart` (siblings idem); `_generate_extension_dart` reescrito para `FletExtension.createWidget`, removendo o caminho `createControl`; `extension.dart` sempre regenerado para casar com as classes geradas.
- pipeline.py: todos os arquivos `.dart` (incl. `extension.dart`) gravados em `lib/src/`.
- test_generators.py: asserts migrados para a convenção `*Control`.
…service
Sobe a faixa de versão do Flet nos templates — `flet>=0.85.1` nos pyproject (extensão + exemplo) e `flet: ^0.85.1` nos pubspec. O caret `^0.80.5` anterior excluía a série 0.85.
Além disso, dois ajustes de qualidade na geração Dart:
- Service static vs instance: `MethodPlan` passa a carregar `is_static` (propagado do parser via analyzer). O gerador instancia o SDK (`final {Main} _{main} = {Main}();`) e despacha métodos de instância nele, mantendo o nome da classe para métodos `static`. Corrige `static_access_to_instance_member` em SDKs de instância (ex.: flutter_secure_storage) sem afetar SDKs estáticos (ex.: OneSignal).
- UI family-variant: o corpo do widget não redeclara mais a variável local `type` quando o pacote tem múltiplos construtores e uma propriedade `type` (corrige `duplicate_definition`/unused em pacotes como lottie).
Validado com `flutter analyze`: erros de static-access e de `type` duplicado eliminados; demais erros restantes são corpo Dart a implementar pelo dev.
…ct dos exemplos (0.85)
- Service generator: substitui a forwarding inválida de named params opcionais (`if (x != null) name: x` dentro da call — collection-if não é válido em argument list) por um único passo em ordem que encaminha named como `name: var` e posicionais como `var`. Valores opcionais já vêm nullable do args map, então o encaminhamento direto compila. Valida no flutter_secure_storage: zero erros de sintaxe (`missing_identifier`/`expected_token` eliminados).
- Templates de exemplo (service + ui_control): remove o bloco inexistente `[tool.flet.dependencies] = { path }` e adiciona a wiring canônica de dev local do Flet 0.85 — `[tool.flet.dev_packages]`, `[tool.uv.sources]` editable e bloco de metadata `[tool.flet]`. Comentário orienta usuários PyPI a remover os blocos de path.
…0.85) - O gerador emitia `await` em toda chamada de SDK, mas métodos void síncronos e getters não retornam Future — await neles dispara await_only_futures e use_of_void_result no `flutter analyze`. Adiciona MethodPlan.dart_is_async (propagado de map_return_type, que já detecta Future<T> e era descartado pelo analyzer) e faz _render_dart_method emitir `await` somente quando a chamada SDK é um Future. Inclui teste de regressão.
…er Dart O regex de prefixo de anotação não consumia a indentação antes da assinatura do método na linha seguinte, então métodos como ` @VisibleForTesting\n static void setMockInitialValues(...)` eram casados a partir de `static` e a anotação ficava fora de group(0) — o filtro nunca disparava e o método de teste vazava para o bridge gerado, quebrando o `flutter analyze`. Ajusta as 4 regexes (método/getter/função top-level/classe) para `(?:@[^\n]*\n\s*)*` e adiciona @Protected aos filtros de skip.
Alguns pacotes embarcam um exemplo `lib/main.dart` (com `main()` top-level + `runApp(...)`) dentro de lib/. Suas classes (MyApp, MyHomePage) não são API pública e vazavam como controles/sibling widgets na extensão UI gerada. Adiciona _is_example_app_file (detecta `main()` top-level) e pula esses arquivos em parse_dart_package_api.
…art gerado
buildWidget/buildWidgets são extension methods em Control (flet/src/extensions/control.dart), então uma chamada sem receiver (`buildWidget("child")`) é indefinida e quebra o flutter analyze. O gerador emitia getters de child/sub-control sem receiver. Passa a emitir `control.buildWidget(...)`/`control.buildWidgets(...)` em type_map e no analyzer, e o gerador reescreve para `widget.control.` pelo caminho normal — removendo as ramificações especiais de buildWidget.
…et 0.85 - Fluxo pós-criação (issue #6): painel "Next Steps" do create.py e seções de teste (about_pkg_flet, README, getting-started) agora orientam `flet build` em vez de `flet run`, explicando o "Unknown control". - Mermaid: habilita pymdownx.superfences custom_fences (mermaid) no mkdocs.yml do projeto e nos templates. architecture.md ganha 4 diagramas (overview, pipeline, classDiagram do modelo de dados, AI refinement); extension-templates e about_pkg_flet convertem a ponte Python<->Dart ASCII para flowchart + sequence. - Atualiza versões 0.80.x->0.85.x (docs/index, extension-templates, badge do README e exemplos embutidos no about_pkg_flet).
…rt gerado control.buildWidget() retorna Widget?, então passá-lo a um parâmetro SDK `required Widget` (ex.: Shimmer.child) quebrava o flutter analyze com argument_type_not_assignable. Adiciona PropertyPlan.dart_type (tipo Dart do param, vindo do analyzer) e o helper _coalesce_child_getter, que acrescenta `?? const SizedBox.shrink()` somente quando o getter é um buildWidget único e o param é não-nulo. Params Widget? são encaminhados como estão; buildWidgets (lista) fica intacto. Inclui test_child_nullability_coalesce.
Import condicional de dart:convert (só quando jsonEncode/jsonDecode é usado), remoção dos helpers _parse{Enum} que nunca eram chamados (dead code), e ordenação dos args do construtor para emitir child/children por último (sort_child_properties_last). Inclui 3 testes de regressão.
multi-linha ft.Color era lido via getString (String? -> Color? quebrava o analyze); passa a usar control.getColor("name", context). E _parse_param agora remove anotações e normaliza whitespace antes de extrair o valor default — um @deprecated multi-linha fazia `bool x = false` ser parseado com nome `false` (gerando `false:` inválido no geolocator). Inclui teste de regressão.
- Params de UI tipados como enum (Axis, WrapAlignment, TextDirection, enums do pacote) caíam no fallback getString -> String? incompatível com o param enum. - Adiciona _FLUTTER_ENUMS + is_known_enum; o analyzer emite parseEnum(<Enum>.values, getString(...)) para enums não-nativos (python_type = enum gerado ou str), e _coalesce_nonnull_getter coalesce com <Enum>.values.first quando o param é não-nulo. - Inclui testes.
Reviewer's GuideAligns generated Flet extensions and tooling with Flet 0.85.x, significantly tightens Dart bridge generation (services and UI controls) to reduce flutter analyze errors, improves Dart parser/analyzer/type mapping, and modernizes docs, templates, and MCP serialization. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7 +/- ##
==========================================
+ Coverage 78.56% 79.07% +0.50%
==========================================
Files 35 35
Lines 4643 4707 +64
==========================================
+ Hits 3648 3722 +74
+ Misses 995 985 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 6 issues, and left some high level feedback:
- In
dart_service.pythe new helpers_dart_arg_extractionand_coalesce_nonnull_getterusere, but the module doesn't importre; add an explicitimport reat the top to avoid a NameError at runtime.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `dart_service.py` the new helpers `_dart_arg_extraction` and `_coalesce_nonnull_getter` use `re`, but the module doesn't import `re`; add an explicit `import re` at the top to avoid a NameError at runtime.
## Individual Comments
### Comment 1
<location path="src/flet_pkg/core/generators/dart_service.py" line_range="1013-1022" />
<code_context>
return "dynamic"
+
+
+def _dart_arg_extraction(key: str, dart_type: str) -> str:
+ """Build the RHS expression to extract a method argument from ``args``.
+
+ JSON arrives as ``List<dynamic>``/``Map<dynamic, dynamic>``, so a typed SDK
+ param like ``List<String>`` needs ``.cast<String>()`` (a plain
+ ``as List<String>`` throws at runtime). Preserves the element types so the
+ SDK call type-checks instead of failing with ``List<dynamic>`` mismatches.
+ """
+ base = dart_type.strip().rstrip("?")
+ inner_match = re.match(r"(List|Map|Set)<(.+)>$", base)
+ if inner_match:
+ outer, inner = inner_match.group(1), inner_match.group(2).strip()
+ if inner and inner != "dynamic" and "dynamic" not in inner:
</code_context>
<issue_to_address>
**suggestion:** Generic argument casting under-handles common `Map<String, dynamic>` and similar cases.
Right now, if the inner type contains `dynamic` you skip `.cast<...>()` and fall back to `as {_dart_cast_type(dart_type)}` (e.g. `as Map?`). For SDK types like `Map<String, dynamic>` or `List<Map<String, dynamic>>`, this loses the declared key/value types and can produce analyzer warnings or force extra casts later. It would be better to special-case these so you still emit `.cast<String, dynamic>()` (and in general `.cast<K, V>()` even when `V` is `dynamic`) to preserve the SDK’s generic shape in the bridge.
</issue_to_address>
### Comment 2
<location path="tests/test_generators.py" line_range="274-275" />
<code_context>
content = files["one_signal_service.dart"]
assert "package:onesignal_flutter/onesignal_flutter.dart" in content
+ def test_no_unused_enum_parser_helpers(self, sample_plan):
+ """Enum parser helpers were dead code (never called) → must not be emitted.
+
+ sample_plan has an OSLogLevel enum; the service must NOT contain a
+ `_parseOSLogLevel` helper (it would be an unused_element in analyze).
+ """
+ content = DartServiceGenerator().generate(sample_plan)["one_signal_service.dart"]
+ assert "_parse" not in content
+
+ def test_typed_list_arg_uses_cast(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Tighten the assertion for unused enum parser helpers to avoid false positives
`assert "_parse" not in content` risks failing if future valid helpers or identifiers contain `_parse`. To avoid brittle false positives while still guarding against the unused helper regression, assert against the specific helper name you expect to be absent (e.g. `"_parseOSLogLevel(" not in content`) or a pattern derived from the enum name in `sample_plan`.
```suggestion
content = DartServiceGenerator().generate(sample_plan)["one_signal_service.dart"]
# Guard specifically against the unused OSLogLevel enum helper, rather than any `_parse` usage.
assert "_parseOSLogLevel(" not in content
```
</issue_to_address>
### Comment 3
<location path="tests/test_generators.py" line_range="341-350" />
<code_context>
+ def test_dart_convert_import_only_when_used(self, sample_plan):
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test covering the `dart:convert` import when only `jsonDecode` (event fields) is used
This already covers `dart:convert` for `jsonEncode`-based returns and the no-usage case. To also cover the `jsonDecode` path for `dict` event fields, please add a small `GenerationPlan` with an event whose field type is `"dict"` and assert that:
- `import 'dart:convert';` is generated,
- the event handler references `jsonDecode`, and
- the import is still generated even if there are no dict/list-returning methods.
That will lock in the behavior and prevent regressions around `jsonDecode` usage being missed.
Suggested implementation:
```python
def test_dart_convert_import_only_when_used(self, sample_plan):
"""`import 'dart:convert'` only when jsonEncode/jsonDecode is actually used."""
gen = DartServiceGenerator()
# Case 1: sample_plan has get_tags() returning dict → jsonEncode → needs convert.
used = gen.generate(sample_plan)["one_signal_service.dart"]
assert "import 'dart:convert';" in used
assert "jsonEncode" in used
# Case 2: A plan with only simple (str/None) returns must NOT import dart:convert.
simple_plan = GenerationPlan(
control_name="Simple",
services=[
Service(
name="SimpleService",
methods=[
Method(
name="ping",
params=[],
return_type="str",
),
Method(
name="maybe",
params=[],
return_type="None",
),
],
events=[],
)
],
)
simple_content = gen.generate(simple_plan)["simple_service.dart"]
assert "import 'dart:convert';" not in simple_content
# Case 3: A plan with an event that has a dict field should still import dart:convert
# and reference jsonDecode, even if there are no dict/list-returning methods.
decode_plan = GenerationPlan(
control_name="Decode",
services=[
Service(
name="DecodeService",
methods=[
Method(
name="noop",
params=[],
return_type="None",
)
],
events=[
Event(
name="Updated",
fields=[
Field(
name="payload",
type="dict",
)
],
)
],
)
],
)
decode_content = gen.generate(decode_plan)["decode_service.dart"]
assert "import 'dart:convert';" in decode_content
assert "jsonDecode" in decode_content
```
To integrate this with your actual codebase, you may need to:
1. Adjust the construction of `GenerationPlan`, `Service`, `Method`, `Event`, and `Field` to match the real generator model classes:
- If they live in a module (e.g. `from yourpkg.generators import Service, Method, Event, Field`), ensure they are imported at the top of `tests/test_generators.py`.
- If different names are used (e.g. `ServicePlan`, `EventSpec`, `FieldSpec`, etc.), substitute them accordingly in the new test body.
2. Ensure that the filenames used in the assertions (`"simple_service.dart"` and `"decode_service.dart"`) match the naming convention used by `DartServiceGenerator().generate(plan)`. If the generator uses a different pattern, update those keys accordingly.
3. If your event type for map-like payloads is not exactly `"dict"` (e.g. `"map"` or a more specific schema descriptor), change `type="dict"` in the `Field` construction to the correct value so that the Dart generator chooses the jsonDecode-based path for that event field.
</issue_to_address>
### Comment 4
<location path="tests/test_generators.py" line_range="755-759" />
<code_context>
+ def test_nonnull_numeric_getter_coalesced(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for non-nullable `String` getters being coalesced
There’s an untested branch in `_coalesce_nonnull_getter` for `getString(...)` → `?? ""`. Please add a small `GenerationPlan` case with a non-nullable `dart_type="String"` property using `control.getString(...)` and assert that it coalesces to `?? ""`, while a `String?` counterpart does not get coalesced. This will fully cover that code path and protect it during future refactors.
```suggestion
content = DartServiceGenerator().generate(plan)["wrapper_control.dart"]
# In the Wrapper(...) constructor call, `padding:` must precede `child:`.
assert content.index("padding: padding,") < content.index("child: child,")
def test_nonnull_string_getter_coalesced(self):
"""Nullable string getters feeding a non-null param must be coalesced.
`getString` returns `String?` — a non-null SDK param needs `?? ""`;
a nullable param is forwarded as-is.
"""
plan = GenerationPlan(
control_name="Gauge",
package_name="flet_gauge",
base_class="ft.LayoutControl",
flutter_package="gauge",
props=[
PropertyGenerationPlan(
dart_name="title",
dart_getter='control.getString("title")',
dart_type="String",
),
PropertyGenerationPlan(
dart_name="subtitle",
dart_getter='control.getString("subtitle")',
dart_type="String?",
),
],
)
content = DartServiceGenerator().generate(plan)["gauge_control.dart"]
# Non-nullable String getter must be coalesced with `?? ""`.
assert 'title: control.getString("title") ?? "",' in content
# Nullable String getter must be forwarded as-is (no coalescing).
assert 'subtitle: control.getString("subtitle"),' in content
def test_nonnull_numeric_getter_coalesced(self):
```
</issue_to_address>
### Comment 5
<location path="tests/test_generators.py" line_range="277-286" />
<code_context>
+ def test_typed_list_arg_uses_cast(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Expand coverage of `_dart_arg_extraction` to Map/Set and non-generic cases
Since `_dart_arg_extraction` now also supports other generic collections and falls back to `_dart_cast_type` for scalars, it would be helpful to add tests that:
- cover a `Map<String, String>` (and optionally `Set<int>`) parameter and assert that the generated Dart uses the expected `.cast<...>()` pattern, and
- cover a non-collection parameter (e.g. `int` or `bool?`) and assert it still uses a simple `as` cast.
You can follow the same pattern as this test by building minimal `GenerationPlan`s and asserting on the exact extraction line.
Suggested implementation:
```python
def test_no_unused_enum_parser_helpers(self, sample_plan):
"""Enum parser helpers were dead code (never called) → must not be emitted.
sample_plan has an OSLogLevel enum; the service must NOT contain a
`_parseOSLogLevel` helper (it would be an unused_element in analyze).
"""
content = DartServiceGenerator().generate(sample_plan)["one_signal_service.dart"]
assert "_parse" not in content
def test_typed_map_arg_uses_cast(self):
"""A Map<String, String> arg must use .cast<K, V>() so the SDK call type-checks."""
gen = DartServiceGenerator()
# When extracting a Map<String, String> argument, we should downcast the dynamic map.
extraction = gen._dart_arg_extraction(
arg_name="metadata",
dart_type="Map<String, String>",
)
# Example expected pattern:
# final metadata = (args['metadata'] as Map).cast<String, String>();
assert "metadata" in extraction
assert "as Map" in extraction
assert ".cast<String, String>()" in extraction
def test_non_collection_arg_uses_as_cast(self):
"""Non-collection args (e.g. int, bool?) must use simple `as` casts, not `.cast`."""
gen = DartServiceGenerator()
# Non-nullable scalar
int_extraction = gen._dart_arg_extraction(
arg_name="retryCount",
dart_type="int",
)
# Example expected pattern:
# final retryCount = args['retryCount'] as int;
assert "retryCount" in int_extraction
assert "as int" in int_extraction
assert ".cast<" not in int_extraction
# Nullable scalar
bool_extraction = gen._dart_arg_extraction(
arg_name="isActive",
dart_type="bool?",
)
# Example expected pattern:
# final isActive = args['isActive'] as bool?;
assert "isActive" in bool_extraction
assert "as bool?" in bool_extraction
assert ".cast<" not in bool_extraction
def test_typed_list_arg_uses_cast(self):
"""A List<String>/Map<String,String> param must use .cast<T>() (JSON is
List<dynamic>) so the SDK call type-checks instead of List<dynamic> mismatch."""
plan = GenerationPlan(
control_name="Tagger",
package_name="flet_tagger",
base_class="ft.Service",
flutter_package="tagger",
dart_import="package:tagger/tagger.dart",
dart_main_class="Tagger",
main_methods=[
```
These tests assume `_dart_arg_extraction` is a method on `DartServiceGenerator` with a signature compatible with:
```python
_dart_arg_extraction(arg_name: str, dart_type: str, ...)
```
You may need to:
1. Adjust the `_dart_arg_extraction` call signature (e.g. if it expects a parameter object instead of `arg_name`/`dart_type` keyword args).
2. Update the string expectations (`"as Map"`, `".cast<String, String>()"`, `"as int"`, `"as bool?"`) to match the exact code your generator emits (for example, it might use `Map<Object?, Object?>` or include `args["..."]` with double quotes).
3. If your test suite prefers validating via `GenerationPlan` and full file generation (like `test_typed_list_arg_uses_cast`), you can instead:
- Build a minimal `GenerationPlan` with methods that have `Map<String, String>`, `Set<int>`, and scalar parameters.
- Generate Dart and assert on the exact extraction lines in the generated service file rather than directly calling `_dart_arg_extraction`.
</issue_to_address>
### Comment 6
<location path="tests/test_parser.py" line_range="59-68" />
<code_context>
+class TestAnnotationFiltering:
</code_context>
<issue_to_address>
**suggestion (testing):** Complement class-method annotation tests with a top-level function case
Since the same annotation filters are applied to top-level functions, please add a test that passes a file with a `@visibleForTesting` or `@protected` top-level function into `parse_dart_package_api` (or the underlying parser) and asserts that the function is omitted from the parsed API. This will confirm the top-level path stays consistent with the class-method behavior and guard against reintroducing internal/test-only functions into the generated bridge.
Suggested implementation:
```python
class TestAnnotationFiltering:
"""Methods annotated as test-only / non-public must be skipped.
The annotations are indented inside a class body and the method signature
sits on the next (also indented) line — the regex must consume that
indentation, otherwise the annotation is dropped from the match and the
method leaks into the generated bridge (breaking `flutter analyze`).
"""
def test_skips_top_level_visible_for_testing(self, tmp_path):
# The same annotation filtering used for class methods should also apply
# to top-level functions fed into `parse_dart_package_api`.
lib_dir = tmp_path / "lib"
lib_dir.mkdir(parents=True, exist_ok=True)
(lib_dir / "top_level.dart").write_text(
"""
import 'package:meta/meta.dart';
@visibleForTesting
void internalHelper() {}
@protected
void internalProtected() {}
void publicHelper() {}
""",
encoding="utf8",
)
api = parse_dart_package_api(tmp_path, include_widgets=False)
function_names = {f.name for f in api.functions}
# Public function should be present.
assert "publicHelper" in function_names
# Test-only / non-public top-level functions must be omitted.
assert "internalHelper" not in function_names
assert "internalProtected" not in function_names
def test_skips_indented_visible_for_testing(self):
src = """
```
If the parsed API object does not expose top-level functions via `api.functions`, adjust `function_names = {f.name for f in api.functions}` to use the appropriate collection/attribute for top-level functions in your real API model (for example, `api.top_level_functions` or similar). All other logic (writing the temporary Dart file via `tmp_path`, calling `parse_dart_package_api`, and asserting the absence of the annotated functions) should remain the same.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1013
to
+1022
| def _dart_arg_extraction(key: str, dart_type: str) -> str: | ||
| """Build the RHS expression to extract a method argument from ``args``. | ||
|
|
||
| JSON arrives as ``List<dynamic>``/``Map<dynamic, dynamic>``, so a typed SDK | ||
| param like ``List<String>`` needs ``.cast<String>()`` (a plain | ||
| ``as List<String>`` throws at runtime). Preserves the element types so the | ||
| SDK call type-checks instead of failing with ``List<dynamic>`` mismatches. | ||
| """ | ||
| base = dart_type.strip().rstrip("?") | ||
| inner_match = re.match(r"(List|Map|Set)<(.+)>$", base) |
There was a problem hiding this comment.
suggestion: Generic argument casting under-handles common Map<String, dynamic> and similar cases.
Right now, if the inner type contains dynamic you skip .cast<...>() and fall back to as {_dart_cast_type(dart_type)} (e.g. as Map?). For SDK types like Map<String, dynamic> or List<Map<String, dynamic>>, this loses the declared key/value types and can produce analyzer warnings or force extra casts later. It would be better to special-case these so you still emit .cast<String, dynamic>() (and in general .cast<K, V>() even when V is dynamic) to preserve the SDK’s generic shape in the bridge.
Comment on lines
+274
to
+275
| content = DartServiceGenerator().generate(sample_plan)["one_signal_service.dart"] | ||
| assert "_parse" not in content |
There was a problem hiding this comment.
suggestion (testing): Tighten the assertion for unused enum parser helpers to avoid false positives
assert "_parse" not in content risks failing if future valid helpers or identifiers contain _parse. To avoid brittle false positives while still guarding against the unused helper regression, assert against the specific helper name you expect to be absent (e.g. "_parseOSLogLevel(" not in content) or a pattern derived from the enum name in sample_plan.
Suggested change
| content = DartServiceGenerator().generate(sample_plan)["one_signal_service.dart"] | |
| assert "_parse" not in content | |
| content = DartServiceGenerator().generate(sample_plan)["one_signal_service.dart"] | |
| # Guard specifically against the unused OSLogLevel enum helper, rather than any `_parse` usage. | |
| assert "_parseOSLogLevel(" not in content |
Comment on lines
+341
to
+350
| def test_dart_convert_import_only_when_used(self, sample_plan): | ||
| """`import 'dart:convert'` only when jsonEncode/jsonDecode is actually used.""" | ||
| gen = DartServiceGenerator() | ||
| # sample_plan has get_tags() returning dict → jsonEncode → needs convert. | ||
| used = gen.generate(sample_plan)["one_signal_service.dart"] | ||
| assert "import 'dart:convert';" in used | ||
| assert "jsonEncode" in used | ||
|
|
||
| # A plan with only simple (str/None) returns must NOT import dart:convert. | ||
| plan = GenerationPlan( |
There was a problem hiding this comment.
suggestion (testing): Add a test covering the dart:convert import when only jsonDecode (event fields) is used
This already covers dart:convert for jsonEncode-based returns and the no-usage case. To also cover the jsonDecode path for dict event fields, please add a small GenerationPlan with an event whose field type is "dict" and assert that:
import 'dart:convert';is generated,- the event handler references
jsonDecode, and - the import is still generated even if there are no dict/list-returning methods.
That will lock in the behavior and prevent regressions around jsonDecode usage being missed.
Suggested implementation:
def test_dart_convert_import_only_when_used(self, sample_plan):
"""`import 'dart:convert'` only when jsonEncode/jsonDecode is actually used."""
gen = DartServiceGenerator()
# Case 1: sample_plan has get_tags() returning dict → jsonEncode → needs convert.
used = gen.generate(sample_plan)["one_signal_service.dart"]
assert "import 'dart:convert';" in used
assert "jsonEncode" in used
# Case 2: A plan with only simple (str/None) returns must NOT import dart:convert.
simple_plan = GenerationPlan(
control_name="Simple",
services=[
Service(
name="SimpleService",
methods=[
Method(
name="ping",
params=[],
return_type="str",
),
Method(
name="maybe",
params=[],
return_type="None",
),
],
events=[],
)
],
)
simple_content = gen.generate(simple_plan)["simple_service.dart"]
assert "import 'dart:convert';" not in simple_content
# Case 3: A plan with an event that has a dict field should still import dart:convert
# and reference jsonDecode, even if there are no dict/list-returning methods.
decode_plan = GenerationPlan(
control_name="Decode",
services=[
Service(
name="DecodeService",
methods=[
Method(
name="noop",
params=[],
return_type="None",
)
],
events=[
Event(
name="Updated",
fields=[
Field(
name="payload",
type="dict",
)
],
)
],
)
],
)
decode_content = gen.generate(decode_plan)["decode_service.dart"]
assert "import 'dart:convert';" in decode_content
assert "jsonDecode" in decode_contentTo integrate this with your actual codebase, you may need to:
-
Adjust the construction of
GenerationPlan,Service,Method,Event, andFieldto match the real generator model classes:- If they live in a module (e.g.
from yourpkg.generators import Service, Method, Event, Field), ensure they are imported at the top oftests/test_generators.py. - If different names are used (e.g.
ServicePlan,EventSpec,FieldSpec, etc.), substitute them accordingly in the new test body.
- If they live in a module (e.g.
-
Ensure that the filenames used in the assertions (
"simple_service.dart"and"decode_service.dart") match the naming convention used byDartServiceGenerator().generate(plan). If the generator uses a different pattern, update those keys accordingly. -
If your event type for map-like payloads is not exactly
"dict"(e.g."map"or a more specific schema descriptor), changetype="dict"in theFieldconstruction to the correct value so that the Dart generator chooses the jsonDecode-based path for that event field.
Comment on lines
+755
to
+759
| content = DartServiceGenerator().generate(plan)["wrapper_control.dart"] | ||
| # In the Wrapper(...) constructor call, `padding:` must precede `child:`. | ||
| assert content.index("padding: padding,") < content.index("child: child,") | ||
|
|
||
| def test_nonnull_numeric_getter_coalesced(self): |
There was a problem hiding this comment.
suggestion (testing): Consider adding a test for non-nullable String getters being coalesced
There’s an untested branch in _coalesce_nonnull_getter for getString(...) → ?? "". Please add a small GenerationPlan case with a non-nullable dart_type="String" property using control.getString(...) and assert that it coalesces to ?? "", while a String? counterpart does not get coalesced. This will fully cover that code path and protect it during future refactors.
Suggested change
| content = DartServiceGenerator().generate(plan)["wrapper_control.dart"] | |
| # In the Wrapper(...) constructor call, `padding:` must precede `child:`. | |
| assert content.index("padding: padding,") < content.index("child: child,") | |
| def test_nonnull_numeric_getter_coalesced(self): | |
| content = DartServiceGenerator().generate(plan)["wrapper_control.dart"] | |
| # In the Wrapper(...) constructor call, `padding:` must precede `child:`. | |
| assert content.index("padding: padding,") < content.index("child: child,") | |
| def test_nonnull_string_getter_coalesced(self): | |
| """Nullable string getters feeding a non-null param must be coalesced. | |
| `getString` returns `String?` — a non-null SDK param needs `?? ""`; | |
| a nullable param is forwarded as-is. | |
| """ | |
| plan = GenerationPlan( | |
| control_name="Gauge", | |
| package_name="flet_gauge", | |
| base_class="ft.LayoutControl", | |
| flutter_package="gauge", | |
| props=[ | |
| PropertyGenerationPlan( | |
| dart_name="title", | |
| dart_getter='control.getString("title")', | |
| dart_type="String", | |
| ), | |
| PropertyGenerationPlan( | |
| dart_name="subtitle", | |
| dart_getter='control.getString("subtitle")', | |
| dart_type="String?", | |
| ), | |
| ], | |
| ) | |
| content = DartServiceGenerator().generate(plan)["gauge_control.dart"] | |
| # Non-nullable String getter must be coalesced with `?? ""`. | |
| assert 'title: control.getString("title") ?? "",' in content | |
| # Nullable String getter must be forwarded as-is (no coalescing). | |
| assert 'subtitle: control.getString("subtitle"),' in content | |
| def test_nonnull_numeric_getter_coalesced(self): |
Comment on lines
+277
to
+286
| def test_typed_list_arg_uses_cast(self): | ||
| """A List<String>/Map<String,String> param must use .cast<T>() (JSON is | ||
| List<dynamic>) so the SDK call type-checks instead of List<dynamic> mismatch.""" | ||
| plan = GenerationPlan( | ||
| control_name="Tagger", | ||
| package_name="flet_tagger", | ||
| base_class="ft.Service", | ||
| flutter_package="tagger", | ||
| dart_import="package:tagger/tagger.dart", | ||
| dart_main_class="Tagger", |
There was a problem hiding this comment.
suggestion (testing): Expand coverage of _dart_arg_extraction to Map/Set and non-generic cases
Since _dart_arg_extraction now also supports other generic collections and falls back to _dart_cast_type for scalars, it would be helpful to add tests that:
- cover a
Map<String, String>(and optionallySet<int>) parameter and assert that the generated Dart uses the expected.cast<...>()pattern, and - cover a non-collection parameter (e.g.
intorbool?) and assert it still uses a simpleascast.
You can follow the same pattern as this test by building minimal GenerationPlans and asserting on the exact extraction line.
Suggested implementation:
def test_no_unused_enum_parser_helpers(self, sample_plan):
"""Enum parser helpers were dead code (never called) → must not be emitted.
sample_plan has an OSLogLevel enum; the service must NOT contain a
`_parseOSLogLevel` helper (it would be an unused_element in analyze).
"""
content = DartServiceGenerator().generate(sample_plan)["one_signal_service.dart"]
assert "_parse" not in content
def test_typed_map_arg_uses_cast(self):
"""A Map<String, String> arg must use .cast<K, V>() so the SDK call type-checks."""
gen = DartServiceGenerator()
# When extracting a Map<String, String> argument, we should downcast the dynamic map.
extraction = gen._dart_arg_extraction(
arg_name="metadata",
dart_type="Map<String, String>",
)
# Example expected pattern:
# final metadata = (args['metadata'] as Map).cast<String, String>();
assert "metadata" in extraction
assert "as Map" in extraction
assert ".cast<String, String>()" in extraction
def test_non_collection_arg_uses_as_cast(self):
"""Non-collection args (e.g. int, bool?) must use simple `as` casts, not `.cast`."""
gen = DartServiceGenerator()
# Non-nullable scalar
int_extraction = gen._dart_arg_extraction(
arg_name="retryCount",
dart_type="int",
)
# Example expected pattern:
# final retryCount = args['retryCount'] as int;
assert "retryCount" in int_extraction
assert "as int" in int_extraction
assert ".cast<" not in int_extraction
# Nullable scalar
bool_extraction = gen._dart_arg_extraction(
arg_name="isActive",
dart_type="bool?",
)
# Example expected pattern:
# final isActive = args['isActive'] as bool?;
assert "isActive" in bool_extraction
assert "as bool?" in bool_extraction
assert ".cast<" not in bool_extraction
def test_typed_list_arg_uses_cast(self):
"""A List<String>/Map<String,String> param must use .cast<T>() (JSON is
List<dynamic>) so the SDK call type-checks instead of List<dynamic> mismatch."""
plan = GenerationPlan(
control_name="Tagger",
package_name="flet_tagger",
base_class="ft.Service",
flutter_package="tagger",
dart_import="package:tagger/tagger.dart",
dart_main_class="Tagger",
main_methods=[These tests assume _dart_arg_extraction is a method on DartServiceGenerator with a signature compatible with:
_dart_arg_extraction(arg_name: str, dart_type: str, ...)You may need to:
- Adjust the
_dart_arg_extractioncall signature (e.g. if it expects a parameter object instead ofarg_name/dart_typekeyword args). - Update the string expectations (
"as Map",".cast<String, String>()","as int","as bool?") to match the exact code your generator emits (for example, it might useMap<Object?, Object?>or includeargs["..."]with double quotes). - If your test suite prefers validating via
GenerationPlanand full file generation (liketest_typed_list_arg_uses_cast), you can instead:- Build a minimal
GenerationPlanwith methods that haveMap<String, String>,Set<int>, and scalar parameters. - Generate Dart and assert on the exact extraction lines in the generated service file rather than directly calling
_dart_arg_extraction.
- Build a minimal
Comment on lines
+59
to
+68
| class TestAnnotationFiltering: | ||
| """Methods annotated as test-only / non-public must be skipped. | ||
|
|
||
| The annotations are indented inside a class body and the method signature | ||
| sits on the next (also indented) line — the regex must consume that | ||
| indentation, otherwise the annotation is dropped from the match and the | ||
| method leaks into the generated bridge (breaking `flutter analyze`). | ||
| """ | ||
|
|
||
| def test_skips_indented_visible_for_testing(self): |
There was a problem hiding this comment.
suggestion (testing): Complement class-method annotation tests with a top-level function case
Since the same annotation filters are applied to top-level functions, please add a test that passes a file with a @visibleForTesting or @protected top-level function into parse_dart_package_api (or the underlying parser) and asserts that the function is omitted from the parsed API. This will confirm the top-level path stays consistent with the class-method behavior and guard against reintroducing internal/test-only functions into the generated bridge.
Suggested implementation:
class TestAnnotationFiltering:
"""Methods annotated as test-only / non-public must be skipped.
The annotations are indented inside a class body and the method signature
sits on the next (also indented) line — the regex must consume that
indentation, otherwise the annotation is dropped from the match and the
method leaks into the generated bridge (breaking `flutter analyze`).
"""
def test_skips_top_level_visible_for_testing(self, tmp_path):
# The same annotation filtering used for class methods should also apply
# to top-level functions fed into `parse_dart_package_api`.
lib_dir = tmp_path / "lib"
lib_dir.mkdir(parents=True, exist_ok=True)
(lib_dir / "top_level.dart").write_text(
"""
import 'package:meta/meta.dart';
@visibleForTesting
void internalHelper() {}
@protected
void internalProtected() {}
void publicHelper() {}
""",
encoding="utf8",
)
api = parse_dart_package_api(tmp_path, include_widgets=False)
function_names = {f.name for f in api.functions}
# Public function should be present.
assert "publicHelper" in function_names
# Test-only / non-public top-level functions must be omitted.
assert "internalHelper" not in function_names
assert "internalProtected" not in function_names
def test_skips_indented_visible_for_testing(self):
src = """If the parsed API object does not expose top-level functions via api.functions, adjust function_names = {f.name for f in api.functions} to use the appropriate collection/attribute for top-level functions in your real API model (for example, api.top_level_functions or similar). All other logic (writing the temporary Dart file via tmp_path, calling parse_dart_package_api, and asserting the absence of the annotated functions) should remain the same.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Align generated Flet extensions and tooling with Flet 0.85.x, improving Dart bridge generation quality, parser/analyzer robustness, and documentation while refining project scaffolding and MCP serialization.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: