Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Example Extractor and Tester Tools Status #646

Closed
20 tasks done
RexJaeschke opened this issue Oct 26, 2022 · 26 comments
Closed
20 tasks done

Example Extractor and Tester Tools Status #646

RexJaeschke opened this issue Oct 26, 2022 · 26 comments
Assignees
Labels
meeting: discuss This issue should be discussed at the next TC49-TG2 meeting type: tools This issue (or PR) relates to a tool, not the Standard

Comments

@RexJaeschke
Copy link
Contributor

RexJaeschke commented Oct 26, 2022

To add your input to the discussion of any category in this issue, do NOT create a new comment; instead, locate the discussion thread comment for that category and add your contribution to the end using the “edit” option.

The ExampleExtractor tool extracts source code for individual examples based on the metadata in an HTML comment that precedes each example’s code block. (If an example has no such comment, or the initial comment metadata is not recognized, that example’s code is not extracted.)

While most examples have metadata that allows them to be extracted, compiled, and optionally executed, without error, some cannot yet be handled completely by the extraction and testing tools. This issue tracks the status of the different categories of support currently provided by, or needed from, the tool. Once support for a category has been provided, that category’s checkbox should be checked.

Issues relating to specific metadata categories are addressed under Categories. General issues are addressed under General Issues and Observations.

There is a User Guide and grammar for the Extraction Metadata format.

Categories

Each category shows the number of occurrences of examples of that category in all md files as of February 14, 2023.

To find all examples that have not yet been completed for extraction, search for $Example: {.

General Issues and Observations

Some issues transcend categories:

@RexJaeschke RexJaeschke added the type: tools This issue (or PR) relates to a tool, not the Standard label Oct 26, 2022
@RexJaeschke
Copy link
Contributor Author

Complete Example

Metadata: <!-- Example: { …

The metadata is correct and complete. The code compiles (and optionally executes), as is. No further action is needed.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Incomplete Example

Metadata: <!-- Incomplete$Example: { …

Example needs support infrastructure such as declarations or an enclosing type. As this is the single biggest group of currently excluded examples, this is the one worth investing some time to see if it can be addressed, at least for most examples.

TODO More research is needed to identify the specific kinds of support needed.

Many have been completed by using the additionalFiles and customEllipsisReplacements directives.

The remaining such examples are stay incomplete, and might never be completed. This will not be a barrier to closing this issue.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Implementation-Defined Behavior

Metadata: <!-- ImplementationDefined$Example: { …

A few examples produce multiple output tokens whose order might vary from one conforming implementation to another, in which case, what to say for expectedOutput? (It’s not clear, but in some cases, the behavior might legitimately vary between executions on the same platform!) Some of these may well be unspecified behavior instead.

Each such example has an HTML comment saying "<!-- Maintenance Note: The behavior of this example is implementation-defined. As such, the metadata does not compare test output with any ```console block."

No further action needed.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Detecting Intended Exceptions

Metadata: <!-- ThrowsException$Example: { …

How to detect and check it’s the expected exception?

2022-10-26: Problem solved by PR #648, by adding a new metadata string/value pair, expectedException:"<unqualified-type-name>".

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Multifile Examples

Metadata: <!-- RequiresSeparateFiles$Example: { …
Metadata: <!-- RequiresSeparateProjects$Example: { …

Two or more source files need to be compiled together.

Some parts of some examples contain preprocessor directives, which must come before declarations, so they can’t be compiled as a single file.

2023-12-14: Jon resolved part of this problem in PR #735, by adding support for multi-file examples in a single project.

Reclassified as Incomplete$Example. No further action needed.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Example Needing Review

Metadata: <!-- NeedsReview$Example: { …

The compilation or execution resulted in unexpected behavior, but which Rex didn’t understand.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Chevron Delimiters Used for Emphasis

Metadata: <!-- Guillemet$Example: { …

In the Expressions chapter, we use «…» notation for emphasis, as follows:

struct Color
{
    public static readonly Color White = new Color(...);
    public static readonly Color Black = new Color(...);
    public Color Complement() {...}
}

class A
{
    public «Color» Color;              // Field Color of type Color

    void F()
    {
        Color = «Color».Black;         // Refers to Color.Black static member
        Color = Color.Complement();  // Invokes Complement() on Color field
    }

    static void G()
    {
        «Color» c = «Color».White;       // Refers to Color.White static member
    }
}

For expository purposes only, within the A class, those occurrences of the Color identifier that reference the Color type are delimited by «...», and those that reference the Color field are not.

Clearly, this code won’t compile. We can either the example omit from the extraction process or remove the delimiters (and the explanation of their usage).

2022-10-26: Problem solved by PR #649, by having the extractor remove all chevron delimiters. Also made the example complete by handling the "not returning a value" in an ellipses context.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Undefined Behavior

Metadata: <!-- Undefined$Example: { …

We can’t say what, if any, output might result; the program could even abort!

Each such example has an HTML comment saying "<!-- Maintenance Note: The behavior of this example is undefined."

No further action needed.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Including Support Files (formerly Documentation Comment Include File)

Metadata: <!-- IncludeFileNeeded$Example: { …

The documentation comments tag <include> needs a corresponding XML file to include.

2022-11-01: Jon addressed this as part of the more general issue of providing supporting types (and such) in separate files. See PR #662.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Unsafe Code

Such examples require csc option -unsafe+, which neither standalone-console nor standalone-lib templates support. As such, we’d likely need unsafe variants of both those templates.

Metadata: <!-- Unsafe$Example: { … (17)
Metadata: <!-- Unsafe/Undefined/$Example: { … (2)
Metadata: <!-- Unsafe/ImplementationDefined/$Example: { … (1)
Metadata: <!-- Unsafe/Incomplete/$Example: { … (1)

Three unsafe examples also have another aspect to consider, as shown above.

2022-10-27: The general problem was solved by PR #650, by enabling unsafe mode for all examples. What remains are:

Metadata: <!-- Undefined$Example: { … (2)
Metadata: <!-- ImplementationDefined$Example: { … (1)
Metadata: <!-- Incomplete$Example: { … (1)

which will be handled under their respective categories.

jskeet added a commit to jskeet/csharpstandard that referenced this issue Oct 26, 2022
This will allow for examples which are just statements.
(Also adds example in 16.1 Arrays to show how to use it.)

Towards dotnet#646
@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Namespace Access

This applies to both Complete and Incomplete examples.

We currently use three different approaches:

  • Have the necessary using-namespace-directive(s) at the start of the example, with no explicit namespace qualification on any references to names in that namespace. (This makes the example complete.) For example:
    using System;
    using System.Threading;Console.WriteLine("…");
    new Thread(new ThreadStart(Thread2)).Start();  
  • Omit the necessary using-namespace-directive(s) at the start of the example, and have explicit namespace qualification on any references to names in some namespace. (This makes the example complete.) For example:
    System.Console.WriteLine("hello, world");
    class ItemList<T> : System.Collections.Generic.List<T> {}
  • Omit the necessary using-namespace-directive(s) from the start of the example, and omit explicit namespace qualification on any references to names in some namespace. (This makes the example incomplete.) For example:
    Console.WriteLine("…");

Approaches 1 and 2 differ in style; however, both work, but we could/should be consistent! However, to complete the examples in Approach 3, we need to choose Approach 1 or 2, or get the equivalent to Approach 1 by having such text be implicitly included as a preamble.

This has been resolved! There are two versions of each template, called xxx and xxx-without-using (as in standalone-console, and standalone-console-without-using). The former contains using directives for all the namespaces needed by examples in the spec. The latter contains none of them! The latter set is used whenever no names need be imported. It is also used by those few examples that begin with #undef or #define, which must precede using directives, and which might then have explicit using directives..

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Ignoring Certain Warnings

Not all warnings are relevant to the purpose of the tester tool. Can/should such warnings be ignored?

For an example in basic-concepts.md, we have the following:

<!-- Example: {template:"standalone-lib", name:"Declarations2",expectedErrors:["CS0136","CS0136"],expectedWarnings:["CS0219","CS0219","CS0219","CS0219","CS0219","CS0219"]} -->

However, the repeated CS0219 is a message having the form, “The variable 'i' is assigned but its value is never used”. As this is not useful to see or to check for in this context, this kind of warning clutters up the process.

The csc command supports option -nowarn<warn list> to disable one or more warnings. (See also options warn and ruleset.)

One approach we might use to support this is to provide new metadata, something like the following:

<!-- Example: { … ,excludedWarnings:["CS0219", …] … } -->

which gets turned into the corresponding -nowarn compiler option. An alternate approach would be to hard-code the list of exclusions in the testing process, or to read them from a configuration file. It seems like we’d want the list of exclusions to apply to all examples, not just to selected ones.


Jon: rather than using nowarn, it's probably just simpler to remove the excluded warnings from the actual ones before comparing them. (This should be trivial with LINQ.)

2022-10-27: Problem solved by PR #651, by the addition of ignoredWarnings:["CSxxxx", ..., "CSxxxx"].

The following warnings have been ignored in all metadata as of 2022-10-27:

CS0168 -- The variable 'var' is declared but never used
CS0169 -- The private field 'class member' is never used
CS0219 -- The variable 'variable' is assigned but its value is never used
CS0414 -- The private field 'field' is assigned but its value is never used
CS0649 -- Field 'field' is never assigned to, and will always have its default value 'value'
CS1591 -- Missing XML comment for publicly visible type or member 'Type_or_Member'

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Problem with Ellipses Replacement

The metadata replaceEllipsis:true causes all ellipses—regardless of context—to be replaced by /* … */. For the most part, this is OK. However, when ellipses are used as the (unstated) body of a non-void method or a get accessor, this results in error CS0161, “not all code paths return a value.” For example:

class Gen<T,U>
{
    public T[,] a;
    public void G(int i, T t, Gen<U,T> gt) {/*...*/}  // OK, void method
    public U Prop { get {/*...*/} set {/*...*/} }     // CS0161, getter; setter OK
    public int H(double d) {/*...*/}                  // CS0161, non-void method
}

The metadata for such examples is marked IncompleteExample.

Separately, one example contained the following:

[Help("http://www.mycompany.com/.../Class1.htm")]

The ellipses was changed to something like “xxx”.

2022-11-09: Jon resolved this in PR #665.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Example Containing Chevron Quotes

We use chevron-quotes («…») to allow grammar-rule (and other) names inside a code block. For example:

{
    ResourceType resource = «expression»;
    IDisposable d = resource;
    try
    {
        «statement»;
    }
    finally
    {
        if (d != null)
        {
            d.Dispose();
        }
    }
}

Obviously, these examples won’t compile. As such, they do not currently have any metadata.

As a result of #646 (comment), chevron delimiters are now removed from extracted source. For the examples in question, each grammar-rule name is a well-formed identifier, which the compiler will see as being undeclared. As such, these examples are not intended to be compiled. No action needed.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Passing Command-Line Arguments to Main

Several examples process command-line argument strings, if they exist. However, one example (Indexers2 in classes.md) expects such an array of at least one element, and throws an exception when that is not passed.

2023-12-14: Jon resolved this in PR #736.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Errors Hiding Other Errors

Example UsingAliasDirectives13 in namespaces.md contains the following lines (where 9-15 are the corresponding source line numbers):

namespace N1
{
    class A<T>
    {
        class B {}
    }
}

9 namespace N2
10{
11     using W = N1.A;       // Error, cannot name unbound generic type
12     using X = N1.A.B;     // Error, cannot name unbound generic type
13     using Y = N1.A<int>;  // Ok, can name closed constructed type
14     using Z<T> = N1.A<T>; // Error, using alias cannot have type parameters
15}

When compiled, we get

Library.cs(14,16): error CS1002: ; expected
Library.cs(14,24): error CS0116: A namespace cannot directly contain members such as fields, methods or statements
Library.cs(14,16): error CS1022: Type or namespace definition, or end-of-file expected
Library.cs(14,25): error CS1022: Type or namespace definition, or end-of-file expected

However, when the offending line 14 is commented-out, we get

Library.cs(11,18): error CS0305: Using the generic type 'A<T>' requires 1 type arguments
Library.cs(12,18): error CS0305: Using the generic type 'A<T>' requires 1 type arguments

So, the error on line 14 hides the detection/reporting of the errors on lines 11 and 12. This means, we’d need to state the first 4 errors in expectedError, and omit the final two. I can imagine a future release of the compiler (or the use of different compiler options) to behave differently.

No action needed!

@RexJaeschke
Copy link
Contributor Author

Expected Documentation Comments Generated XML

The csc option -doc:xx.xml, saves the generated XML. At this stage, I don’t see that we need/want to compare that with some sort of expectedXMLOutput metadata facility.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Oct 26, 2022

Examples Not Marked for Extraction

These examples are mostly isolated code fragments, contain superscripts or subscripts, or for some other reason, did not warrant trying to make them complete.

No action required.

jskeet added a commit to jskeet/csharpstandard that referenced this issue Oct 26, 2022
Only the name (without the namespace) is specified; the FQN could be quite long-winded in the metadata.
Towards dotnet#646.
jskeet added a commit to jskeet/csharpstandard that referenced this issue Oct 26, 2022
RexJaeschke pushed a commit that referenced this issue Oct 26, 2022
This will allow for examples which are just statements.
(Also adds example in 16.1 Arrays to show how to use it.)

Towards #646
RexJaeschke pushed a commit that referenced this issue Oct 26, 2022
…#648)

Only the name (without the namespace) is specified; the FQN could be quite long-winded in the metadata.
Towards #646.
jskeet added a commit to jskeet/csharpstandard that referenced this issue Oct 27, 2022
@RexJaeschke
Copy link
Contributor Author

Using Program Console Output in Spec

The output from some examples is shown in the spec using the following construct:

```console
<output lines go here>
```

It would be better to use that for checking instead of having an expectedOutput annotation directive.

2022-11-01: Jon added this capability via the inferOutput annotation directive, in PR #661.

@RexJaeschke
Copy link
Contributor Author

RexJaeschke commented Jan 13, 2023

External Reference

Metadata: <!-- ExternalRef$Example: { …

The example contains extern alias.

I got a version to work outside the test framework. For example, for test "UsingAliasDirectives3" in namepsaces.md

  1. We create the external definitions in files ExternAliasX.cs and ExternAliasY.cs in the additional-files folder.
  2. We need a DLL for each of these compiled using csc ExternAliasX.cs -t:library and csc ExternAliasY.cs -t:library
  3. The Library.cs file needs to be compiled using csc Library.cs -t:library -r:X=ExternAliasX.dll -r:Y=ExternAliasY.dll, where the two references must include the path to the DLLs. -->

2023-01-16: Notes from Jon:

We don't compile using csc directly, but with project files, so we'd need to work out what these look like in project files. Extern aliases are more complex than I thought, though given that:

  • We only build a single project (at least by default)
  • These aren't project properties (in the project file) - they're part of the project reference

This is going to be tricky - and possibly even more complex than is worthwhile.

2023-01: Jon resolved this by adding the template extern-lib and annotation-directive project.

@jskeet
Copy link
Contributor

jskeet commented Mar 27, 2024

Update 2024-03-27: grep '\$Example' *.md finds:

  • Incomplete$Example * 17
  • NeedsReview$Example * 1
  • RequiresSeparateProjects$Example * 2

I'll create a PR to see what I can do with these.

jskeet added a commit to jskeet/csharpstandard that referenced this issue Mar 27, 2024
jskeet added a commit that referenced this issue Mar 27, 2024
@jskeet jskeet added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Mar 27, 2024
@jskeet
Copy link
Contributor

jskeet commented Mar 27, 2024

We're now down to 6 instances of Incomplete$Example, all of which would be really fiddly to fix. I propose we standard on a single comment format, e.g. "UntestedExample: (reason)", update all the remaining ones, and close this issue.

@RexJaeschke
Copy link
Contributor Author

@jon I agree that we can/should close this issue even though not all issues have been resolved. Here are the results of my research earlier today:

  1. I prefer to leave the (6) Incomplete$Examples marked exactly that way, as they really are incomplete (and might never be completed, and we can add that to the description for that category), and it will be useful to distinguish those examples from those that really have not yet been tested. That said, over time, it is more likely we'll have other untested examples because they'll get added during the merging of a PR that adds a new example, which has no extraction/testing notation.
  2. Re Implementation-defined$Example, this no longer exists as a marker. Instead, each such example has the following HTML comment saying "<!-- Maintenance Note: The behavior of this example is implementation-defined. As such, the metadata does not compare test output with any ```console block. -->" So, we can close-out that category.
  3. Re Undefined$Example, this no longer exists as a marker. Instead, each such example has the following HTML comment saying "<!-- Note: the behavior of this example is undefined. -->" I suggest making this say "Maintenance Note" like the previous bullet." Then we can close-out that category.
  4. The remaining NeedsReview$Example (example named "InterfaceMemberAccess1" in interfaces.md) needs attention. The first line of method test is x.Count(1); // Error, however, the tester does not diagnose that. Should it, or is it not an error? The second line is x.Count = 1; // Error, for which the tester reports "Line 18: CS1656: Cannot assign to 'Count' because it is a 'method group'" If we're happy with that, then I'll add expectedError CS1656 for that line.
  5. classes.md contains two examples marked RequiresSeparateProjects$Example. Can those be resolved easily or should they be marked Incomplete$Examples instead?

@jskeet
Copy link
Contributor

jskeet commented Mar 30, 2024

4 - that's what #1062 is about.

5 - no, they can't be resolved easily. I'm happy for them to be changed to Incomplete$Example.

@jskeet
Copy link
Contributor

jskeet commented Mar 30, 2024

(I note that I had originally written a larger explanation of the remaining incomplete examples etc - somehow I managed to lose that comment.)

@RexJaeschke
Copy link
Contributor Author

@jon I just updated this issue to reflect that we are done with it, and I have ticked all the remaining boxes. Now, I'll create a PR with the corresponding edits to the examples, and once you have approved/merged that, you can close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meeting: discuss This issue should be discussed at the next TC49-TG2 meeting type: tools This issue (or PR) relates to a tool, not the Standard
Projects
None yet
Development

No branches or pull requests

2 participants