Skip to content

Commit 5bb9a75

Browse files
committed
Enhance the 'identifier_prefixes' rule to handle prefix exclusivity
Before this patch, prefix exclusivity violations were reported with the same message as the one used when an identifier doesn't start with the required prefix. With this patch, exclusivity violations are now reported with a more understandable message.
1 parent e50310f commit 5bb9a75

File tree

5 files changed

+186
-131
lines changed

5 files changed

+186
-131
lines changed

lkql_checker/share/lkql/identifier_prefixes.lkql

Lines changed: 127 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,9 @@ fun identifier_prefixes(
183183
|" Exc_2 : exception; -- FLAG
184184
|" end Foo;
185185
{
186-
val str_prefix = " does not start with ";
187-
188186
fun check_exclusive(
189-
str,
187+
name,
188+
expected_prefix,
190189
type_exclusive=true,
191190
concurrent_exclusive=true,
192191
access_exclusive=true,
@@ -196,26 +195,75 @@ fun identifier_prefixes(
196195
exception_exclusive=true,
197196
enum_exclusive=true
198197
) =
199-
exclusive
200-
and (
201-
(type_exclusive and str.starts_with(type))
202-
or (concurrent_exclusive and str.starts_with(concurrent))
203-
or (access_exclusive and str.starts_with(access))
204-
or (class_access_exclusive and str.starts_with(class_access))
205-
or (subprogram_access_exclusive and str.starts_with(subprogram_access))
206-
or (constant_exclusive and str.starts_with(constant))
207-
or (exception_exclusive and str.starts_with(exception))
208-
or (enum_exclusive and str.starts_with(enum))
209-
);
198+
|" If the ``exclusive`` rule's parameter is ``true``, this function
199+
|" checks whether ``str`` starts with one of the provided prefixes and
200+
|" returns a diagnostic string if so. Else it return unit.
201+
|" Use ``xxx_exclusive`` parameters to exempt checking on a precise
202+
|" prefix.
203+
if exclusive
204+
then [
205+
name.text & " is not " & s[3] & " but starts with " & s[2]
206+
for s in [
207+
(concurrent_exclusive, concurrent, "a concurrent"),
208+
(class_access_exclusive, class_access, "an access-to-class"),
209+
(subprogram_access_exclusive, subprogram_access, "an access-to-subprogram"),
210+
(access_exclusive, access, "an access"),
211+
(type_exclusive, type, "a type"),
212+
(constant_exclusive, constant, "a constant"),
213+
(exception_exclusive, exception, "an exception"),
214+
(enum_exclusive, enum, "an enumeration")
215+
]
216+
if (
217+
s[1]
218+
and name.f_name.text.starts_with(s[2])
219+
and not expected_prefix.starts_with(s[2])
220+
)
221+
]?[1]
222+
else ();
210223

211-
fun check_enum(str) =
224+
fun check_prefix_and_exclusive(
225+
name,
226+
expected_prefix,
227+
prefix_kind,
228+
check_previous_part_on=null,
229+
type_exclusive=true,
230+
concurrent_exclusive=true,
231+
access_exclusive=true,
232+
class_access_exclusive=true,
233+
subprogram_access_exclusive=true,
234+
constant_exclusive=true,
235+
exception_exclusive=true,
236+
enum_exclusive=true
237+
) =
212238
if (
213-
(enum != "-" and not str.starts_with(enum))
214-
or check_exclusive(str, enum_exclusive=false)
239+
if check_previous_part_on is not null
240+
then check_previous_part_on.p_previous_part() is (null | IncompleteTypeDecl)
241+
) then (
242+
if expected_prefix != "-" and not name.f_name.text.starts_with(expected_prefix)
243+
then name.text & " does not start with " & prefix_kind & " prefix " & expected_prefix
244+
else check_exclusive(
245+
name,
246+
expected_prefix,
247+
type_exclusive,
248+
concurrent_exclusive,
249+
access_exclusive,
250+
class_access_exclusive,
251+
subprogram_access_exclusive,
252+
constant_exclusive,
253+
exception_exclusive,
254+
enum_exclusive
255+
)
215256
)
216-
then str_prefix & "enumeration prefix " & enum
217257
else ();
218258

259+
fun check_enum(name) =
260+
check_prefix_and_exclusive(
261+
name,
262+
enum,
263+
"enumeration",
264+
enum_exclusive=false
265+
);
266+
219267
@memoized
220268
fun get_derived(t) = {
221269
|" Return the element from derived corresponding to t, if any, or ""
@@ -225,7 +273,7 @@ fun identifier_prefixes(
225273
if not res then "" else res[res.length]
226274
};
227275

228-
fun check_prefix(name) =
276+
fun check_name(name) =
229277
|" Given a DefiningName, check that it is validating all expressed
230278
|" prefix constraints. Returns a string containing the message
231279
|" explaining the constraint violation if the defining name is
@@ -237,14 +285,13 @@ fun identifier_prefixes(
237285
when p is not SingleTaskTypeDecl
238286
and concurrent != "-"
239287
=>
240-
if (
241-
p.p_previous_part() is (null | IncompleteTypeDecl) and (
242-
not name.f_name.text.starts_with(concurrent)
243-
or check_exclusive(name.f_name.text, concurrent_exclusive=false)
244-
)
288+
check_prefix_and_exclusive(
289+
name,
290+
concurrent,
291+
"concurrent",
292+
check_previous_part_on=p,
293+
concurrent_exclusive=false
245294
)
246-
then str_prefix & "concurrent prefix " & concurrent
247-
else ()
248295

249296
# 'Class access
250297
| (
@@ -254,14 +301,13 @@ fun identifier_prefixes(
254301
)
255302
) when class_access != "-"
256303
=>
257-
if (
258-
p.p_previous_part() is (null | IncompleteTypeDecl) and (
259-
not name.f_name.text.starts_with(class_access)
260-
or check_exclusive(name.f_name.text, class_access_exclusive=false)
261-
)
304+
check_prefix_and_exclusive(
305+
name,
306+
class_access,
307+
"access-to-class",
308+
check_previous_part_on=p,
309+
class_access_exclusive=false
262310
)
263-
then str_prefix & "access-to-class prefix " & class_access
264-
else ()
265311

266312
# Subprogram access
267313
| (
@@ -271,14 +317,13 @@ fun identifier_prefixes(
271317
)
272318
) when subprogram_access != "-"
273319
=>
274-
if (
275-
p.p_previous_part() is (null | IncompleteTypeDecl) and (
276-
not name.f_name.text.starts_with(subprogram_access)
277-
or check_exclusive(name.f_name.text, subprogram_access_exclusive=false)
278-
)
320+
check_prefix_and_exclusive(
321+
name,
322+
subprogram_access,
323+
"access-to-subprogram",
324+
check_previous_part_on=p,
325+
subprogram_access_exclusive=false
279326
)
280-
then str_prefix & "access-to-subprogram prefix " & subprogram_access
281-
else ()
282327

283328
# Other access types
284329
| (
@@ -288,22 +333,18 @@ fun identifier_prefixes(
288333
)
289334
) when access != "-"
290335
=>
291-
if (
292-
p.p_previous_part() is (null | IncompleteTypeDecl) and (
293-
not name.f_name.text.starts_with(access)
294-
or check_exclusive(
295-
name.f_name.text,
296-
access_exclusive=false,
297-
# If both an Access prefix and a Type prefix are
298-
# set and the type prefix is a prefix of the access
299-
# prefix, we don't want to flag this access because
300-
# it broke the exclusivity of the type prefix.
301-
type_exclusive=false
302-
)
303-
)
336+
check_prefix_and_exclusive(
337+
name,
338+
access,
339+
"access",
340+
check_previous_part_on=p,
341+
# If both an Access prefix and a Type prefix are set and the
342+
# type prefix is a prefix of the access prefix, we don't
343+
# want to flag this access because it broke the exclusivity
344+
# of the type prefix.
345+
access_exclusive=false,
346+
type_exclusive=false
304347
)
305-
then str_prefix & "access prefix " & access
306-
else ()
307348

308349
# (Sub)Types derived from `derived`
309350
| p@(TypeDecl(f_type_def: DerivedTypeDef) | SubtypeDecl)
@@ -312,14 +353,14 @@ fun identifier_prefixes(
312353
when get_derived(t) != ""
313354
=> {
314355
val derived_res = get_derived(p.p_canonical_type().p_base_type());
315-
if (
316-
p.p_previous_part() is (null | IncompleteTypeDecl) and (
317-
derived_res != ""
318-
and not name.f_name.text.starts_with(derived_res.split(":")[2])
319-
)
356+
if derived_res != ""
357+
then check_prefix_and_exclusive(
358+
name,
359+
derived_res.split(":")[2],
360+
"derived",
361+
check_previous_part_on=p
320362
)
321-
then str_prefix & "derived prefix " & derived_res.split(":")[2]
322-
else ()
363+
else check_exclusive(name, "")
323364
}
324365

325366
# Exclude IncompleteTypeDecl
@@ -329,17 +370,16 @@ fun identifier_prefixes(
329370
| p@BaseTypeDecl
330371
when p is not SingleTaskTypeDecl
331372
=>
332-
if (
333-
p.p_previous_part() is (null | IncompleteTypeDecl) and (
334-
(type != "-" and not name.f_name.text.starts_with(type))
335-
or check_exclusive(name.f_name.text, type_exclusive=false)
336-
)
373+
check_prefix_and_exclusive(
374+
name,
375+
type,
376+
"subtype",
377+
check_previous_part_on=p,
378+
type_exclusive=false
337379
)
338-
then str_prefix & "subtype prefix " & type
339-
else ()
340380

341381
# Enums
342-
| EnumLiteralDecl => check_enum(name.f_name.text)
382+
| EnumLiteralDecl => check_enum(name)
343383

344384
# Look one level up for remaining cases
345385
| p => match p.parent
@@ -348,50 +388,45 @@ fun identifier_prefixes(
348388
ObjectDecl(p_is_constant_object(): true)
349389
| NumberDecl
350390
) =>
351-
if (
352-
name.p_previous_part() is null and (
353-
(constant != "-" and not name.f_name.text.starts_with(constant))
354-
or check_exclusive(name.f_name.text, constant_exclusive=false)
355-
)
391+
check_prefix_and_exclusive(
392+
name,
393+
constant,
394+
"constant",
395+
check_previous_part_on=name,
396+
constant_exclusive=false
356397
)
357-
then str_prefix & "constant prefix " & constant
358-
else ()
359-
360398

361399
# Function renaming an enum literal
362400
| r@SubpRenamingDecl
363401
when r.f_renames?.f_renamed_object?.p_referenced_decl?() is EnumLiteralDecl
364402
=>
365-
check_enum(name.f_name.text)
403+
check_enum(name)
366404

367405
# Exceptions
368406
| ExceptionDecl =>
369-
if (
370-
(exception != "-" and not name.f_name.text.starts_with(exception))
371-
or check_exclusive(name.f_name.text, exception_exclusive=false)
407+
check_prefix_and_exclusive(
408+
name,
409+
exception,
410+
"exception",
411+
exception_exclusive=false
372412
)
373-
then str_prefix & "exception prefix " & exception
374-
else ()
375413

376414
# Avoid checking declaration completions
377415
| p@(BodyNode | SubpRenamingDecl | ObjectDecl) => {
378416
val n = if p is ObjectDecl then name else p;
379-
if n.p_previous_part() is null and check_exclusive(name.f_name.text)
380-
then " does not have an exclusive prefix"
417+
if n.p_previous_part() is (null | IncompleteTypeDecl)
418+
then check_exclusive(name, "")
381419
else ()
382420
}
383421

384422
# All other cases, check the exclusivity
385-
| * =>
386-
if check_exclusive(name.f_name.text)
387-
then " does not have an exclusive prefix"
388-
else ();
423+
| * => check_exclusive(name, "");
389424

390425
# Iterate over all defining names and check prefixes for each
391426
[
392-
{message: s[1].text & s[2], loc: s[1]}
427+
{message: s[2], loc: s[1]}
393428
for s in [
394-
(n, check_prefix(n))
429+
(n, check_name(n))
395430
for n in from unit.root select DefiningName
396431
]
397432
if s[2] != ()

lkql_checker/src/gnatcheck-rules.adb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,7 +1038,10 @@ package body Gnatcheck.Rules is
10381038

10391039
function Id_Prefix_Param_From_Diag (Diag : String) return String is
10401040
begin
1041-
if Index (Diag, "task") /= 0 or else Index (Diag, "protected") /= 0 then
1041+
if Index (Diag, "but starts with") /= 0 then
1042+
return "exclusive";
1043+
elsif Index (Diag, "task") /= 0 or else Index (Diag, "protected") /= 0
1044+
then
10421045
return "concurrent";
10431046
elsif Index (Diag, "access-to-class") /= 0 then
10441047
return "class_access";
@@ -1056,8 +1059,6 @@ package body Gnatcheck.Rules is
10561059
return "access";
10571060
elsif Index (Diag, "subtype") /= 0 then
10581061
return "type";
1059-
elsif Index (Diag, "exclusive") /= 0 then
1060-
return "exclusive";
10611062
else
10621063
return "";
10631064
end if;

testsuite/tests/checks/identifier_prefixes/prefix.ads

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
-- +RIdentifier_Prefixes:
2-
-- Type=Type_,Constant=Const_,Exception=X_,Enum=E_,Derived=Prefix.Root:Root_,
3-
-- Class_Access=CA_
4-
51
package Prefix is
62
-- type
73
type Root; -- NOFLAG
@@ -40,6 +36,7 @@ package Prefix is
4036
-- derived type
4137
type Root_1 is new Root;
4238
type Root2 is new Root; -- FLAG
39+
type Root_1 is new Integer; -- FLAG
4340

4441
-- derived type via subtype
4542
subtype Type_Sub1 is Root; -- NOFLAG
@@ -48,22 +45,26 @@ package Prefix is
4845
type Root4 is new Type_Sub2; -- FLAG
4946

5047
-- concurrent type
51-
task type Type_Task is -- NOFLAG
48+
task type Type_Conc_Task is -- NOFLAG
5249
entry E;
5350
end Type_Task;
5451

5552
task type My_Task is -- FLAG
5653
entry E;
5754
end My_Task;
5855

59-
protected type Type_Protected is -- NOFLAG
56+
protected type Type_Conc_Protected is -- NOFLAG
6057
entry E;
6158
end Type_Protected;
6259

6360
protected type My_Protected is -- FLAG
6461
entry E;
6562
end My_Protected;
6663

64+
type Type_Conc_Rec is null record; -- FLAG
65+
type Type_Conc_Der is new Integer; -- FLAG
66+
67+
Const_Or_Node : Integer := 1; -- FLAG
6768
private
6869
type Priv is new Integer; -- NOFLAG: completion
6970
Private_Const : constant Priv := 0; -- NOFLAG: completion

0 commit comments

Comments
 (0)