Skip to content

Commit 81fe0f6

Browse files
authored
Prevent implicit Valkey.Glide type resolution in example validation (valkey-io#367)
Signed-off-by: currantw <taylor.curran@improving.com>
1 parent 735fcd5 commit 81fe0f6

3 files changed

Lines changed: 69 additions & 57 deletions

File tree

scripts/validate_examples.py

Lines changed: 64 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,8 @@
2424
Options:
2525
--examples Path to an existing JSON file containing examples to validate.
2626
--glide-dll Path to the built Valkey.Glide.dll to reference during compilation.
27-
--add-imports Include default using directives (Valkey.Glide namespaces) in
28-
the generated classes. Disabled by default.
29-
--add-clients Include static client fields (GlideClient, GlideClusterClient,
30-
IDatabase, IServer) in the generated classes. Disabled by default.
27+
--add-imports Include default using directives in the generated classes. Disabled by default.
28+
--add-clients Include static client fields in the generated classes. Disabled by default.
3129
"""
3230

3331
import argparse
@@ -55,12 +53,12 @@ class ExamplesValidator:
5553

5654
# Type imports (using static) to include when --add-imports is specified.
5755
_USING_TYPES = [
56+
"Valkey.Glide.Commands.Options.BitFieldOptions.Encoding",
57+
"Valkey.Glide.Commands.Options.BitFieldOptions",
58+
"Valkey.Glide.Commands.Options.InfoOptions",
5859
"Valkey.Glide.Pipeline.Batch",
5960
"Valkey.Glide.Pipeline.ClusterBatch",
60-
"Valkey.Glide.Commands.Options.InfoOptions",
61-
"Valkey.Glide.Commands.Options.BitFieldOptions",
62-
"Valkey.Glide.Commands.Options.BitFieldOptions.Encoding",
63-
"Valkey.Glide.ServerModules.GlideJson",
61+
"Valkey.Glide.Pipeline.Options",
6462
]
6563

6664
# Client fields to include when --add-clients is specified.
@@ -97,7 +95,7 @@ class ExamplesValidator:
9795
9896
{usings}
9997
100-
namespace Valkey.Glide.ExampleValidation;
98+
namespace ExampleValidation;
10199
102100
public class {class_name}
103101
{{
@@ -136,9 +134,10 @@ def __init__(
136134
add_imports: If True, include default using directives in the generated class.
137135
add_clients: If True, include static client fields in the generated class.
138136
"""
139-
self._examples = examples
140-
self._add_imports = add_imports
141-
self._add_clients = add_clients
137+
self._examples: dict[str, str] = examples
138+
self._add_imports: bool = add_imports
139+
self._add_clients: bool = add_clients
140+
142141
self._temp_dir = tempfile.TemporaryDirectory()
143142
self._glide_dll_path = os.path.abspath(glide_dll)
144143

@@ -169,56 +168,66 @@ def validate(self) -> dict[str, list[str]]:
169168
"""Validate the examples by compiling them.
170169
171170
Returns:
172-
A dict mapping source references to lists of error messages for
173-
examples that failed compilation. An empty dict means all examples
174-
compiled successfully.
171+
A dict mapping source references to lists of error messages.
172+
An empty dict means all examples were successfully validated.
175173
"""
176174
if not self._examples:
177175
return {}
178176

179-
file_to_source = self._create_project()
180-
return self._build_project(file_to_source)
177+
self._file_to_source: dict[str, str] = {}
178+
self._source_to_errors: dict[str, list[str]] = {}
179+
180+
self._create_project()
181+
self._build_project()
182+
183+
return self._source_to_errors
181184

182-
def _create_project(self) -> dict[str, str]:
185+
def _create_project(self) -> None:
183186
"""Generate the .csproj and wrapper files in the temp directory."""
184187
csproj_content = self._PROJECT_TEMPLATE.format(dll_path=self._glide_dll_path)
185188

186189
with open(os.path.join(self._temp_dir.name, "ExampleValidation.csproj"), "w") as f:
187190
f.write(csproj_content)
188191

189-
file_to_source: dict[str, str] = {}
190192
for source, content in self._examples.items():
191-
filename = self._generate_class(source, content)
192-
file_to_source[filename] = source
193+
self._generate_class(source, content)
193194

194-
return file_to_source
195-
196-
def _generate_class(self, source: str, content: str) -> str:
195+
def _generate_class(self, source: str, content: str) -> None:
197196
"""Generate and write a C# class for a single example."""
198197

198+
using_directives = []
199+
199200
# Extract 'using' directives from code.
200-
usings = []
201201
code_lines = []
202202
for line in content.splitlines():
203203
if self._USING_DIRECTIVE.match(line):
204-
usings.append(line)
204+
using_directives.append(line)
205205
else:
206206
code_lines.append(line)
207207

208+
# Add default 'using' directives.
208209
if self._add_imports:
209-
usings = (
210-
[f"using {ns};" for ns in self._USING_NAMESPACES]
211-
+ [f"using static {t};" for t in self._USING_TYPES]
212-
+ usings
213-
)
210+
using_directives += [f"using {ns};" for ns in self._USING_NAMESPACES]
211+
using_directives += [f"using static {t};" for t in self._USING_TYPES]
212+
213+
# Detect duplicate using directives.
214+
using_directives_set: set[str] = set()
215+
for u in using_directives:
216+
normalized = u.strip()
217+
if normalized in using_directives_set:
218+
self._source_to_errors.setdefault(source, []).append(
219+
f"Duplicate import: {normalized}"
220+
)
221+
else:
222+
using_directives_set.add(normalized)
214223

215224
# Get fields to include
216225
fields = []
217226
if self._add_clients:
218227
fields += self._CLIENT_FIELDS
219228

220229
class_name = f"Example_{uuid.uuid4().hex}"
221-
usings_str = "\n".join(dict.fromkeys(usings))
230+
usings_str = "\n".join(using_directives_set)
222231
fields_str = "\n".join(f" {f}" for f in fields)
223232
indented_code = textwrap.indent("\n".join(code_lines).strip(), " ")
224233

@@ -235,10 +244,10 @@ def _generate_class(self, source: str, content: str) -> str:
235244
with open(filepath, "w") as f:
236245
f.write(file_content)
237246

238-
return filename
247+
self._file_to_source[filename] = source
239248

240-
def _build_project(self, file_to_source: dict[str, str]) -> dict[str, list[str]]:
241-
"""Run dotnet build and return errors mapped by source."""
249+
def _build_project(self) -> None:
250+
"""Run dotnet build and record compilation errors."""
242251
result = subprocess.run(
243252
["dotnet", "build", "--framework", "net8.0"],
244253
cwd=self._temp_dir.name,
@@ -248,26 +257,32 @@ def _build_project(self, file_to_source: dict[str, str]) -> dict[str, list[str]]
248257
text=True,
249258
)
250259

260+
# Build succeeded:
251261
if result.returncode == 0:
252-
return {}
262+
return
253263

254-
errors: dict[str, list[str]] = {}
264+
# Build failed with matching errors:
265+
found_errors = False
255266
for line in result.stdout.splitlines():
256267
match = self._ERROR_PATTERN.search(line)
257268
if match:
269+
found_errors = True
258270
basename = os.path.basename(match.group("file"))
259-
source = file_to_source.get(basename, basename)
260-
errors.setdefault(source, []).append(match.group("message").strip())
261-
262-
if not errors:
263-
fallback = [f"dotnet build failed with exit code {result.returncode}."]
264-
raw = result.stdout.strip()
265-
if raw:
266-
fallback.append("Raw build output:")
267-
fallback.extend(raw.splitlines())
268-
errors["<build>"] = fallback
269-
270-
return errors
271+
source = self._file_to_source.get(basename, basename)
272+
self._source_to_errors.setdefault(source, []).append(
273+
match.group("message").strip()
274+
)
275+
276+
if found_errors:
277+
return
278+
279+
# Build failed without any matching errors:
280+
fallback = [f"dotnet build failed with exit code {result.returncode}."]
281+
raw = result.stdout.strip()
282+
if raw:
283+
fallback.append("Raw build output:")
284+
fallback.extend(raw.splitlines())
285+
self._source_to_errors.setdefault("<build>", []).extend(fallback)
271286

272287

273288
def main():

sources/Valkey.Glide/Commands/IGenericClusterCommands.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,7 @@ public interface IGenericClusterCommands
176176
/// <example>
177177
/// <code>
178178
/// // Example 1: Atomic Batch (Transaction) all keys must share the same hash slot
179-
/// Pipeline.Options.ClusterBatchOptions options = new(
180-
/// timeout: 1000); // Set a timeout of 1000 milliseconds
179+
/// var options = new ClusterBatchOptions(timeout: 1000); // Set a timeout of 1000 milliseconds
181180
///
182181
/// var batch = new ClusterBatch(true) // Atomic (Transaction)
183182
/// .SetAsync("key", "1")
@@ -191,8 +190,8 @@ public interface IGenericClusterCommands
191190
/// <example>
192191
/// <code>
193192
/// // Example 2: Non-Atomic Batch (Pipeline)
194-
/// var retryStrategy = new Pipeline.Options.ClusterBatchRetryStrategy(retryServerError: true, retryConnectionError: false);
195-
/// Pipeline.Options.ClusterBatchOptions options = new(retryStrategy: retryStrategy);
193+
/// var retryStrategy = new ClusterBatchRetryStrategy(retryServerError: true, retryConnectionError: false);
194+
/// var options = new ClusterBatchOptions(retryStrategy: retryStrategy);
196195
///
197196
/// var batch = new ClusterBatch(false) // Non-Atomic (Pipeline) keys may span different hash slots
198197
/// .SetAsync("key1", "value1")

sources/Valkey.Glide/Commands/IGenericCommands.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,7 @@ public interface IGenericCommands
102102
/// <example>
103103
/// <code>
104104
/// // Example 1: Atomic Batch (Transaction)
105-
/// Pipeline.Options.BatchOptions options = new(
106-
/// timeout: 1000); // Set a timeout of 1000 milliseconds
105+
/// var options = new BatchOptions(timeout: 1000); // Set a timeout of 1000 milliseconds
107106
///
108107
/// var batch = new Batch(true) // Atomic (Transaction)
109108
/// .SetAsync("key", "1")
@@ -117,8 +116,7 @@ public interface IGenericCommands
117116
/// <example>
118117
/// <code>
119118
/// // Example 2: Non-Atomic Batch (Pipeline)
120-
/// Pipeline.Options.BatchOptions options = new(
121-
/// timeout: 1000); // Set a timeout of 1000 milliseconds
119+
/// var options = new BatchOptions(timeout: 1000); // Set a timeout of 1000 milliseconds
122120
///
123121
/// var batch = new Batch(false) // Non-Atomic (Pipeline) keys may span different hash slots
124122
/// .SetAsync("key1", "value1")

0 commit comments

Comments
 (0)