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

Macro-decorated properties are documented with the wrong name. #373

Open
groue opened this issue Jul 16, 2013 · 18 comments
Open

Macro-decorated properties are documented with the wrong name. #373

groue opened this issue Jul 16, 2013 · 18 comments

Comments

@groue
Copy link
Contributor

groue commented Jul 16, 2013

Hi,

This issue is as side effect of CocoaPods/CocoaPods#1204.

The following method, declared at https://github.com/groue/GRMustache/blob/v6.7.4/src/classes/GRMustacheConfiguration.h#L157:

@property (nonatomic, retain) GRMustacheContext *baseContext AVAILABLE_GRMUSTACHE_VERSION_6_4_AND_LATER;

Is outputted by appledoc as (see for instance http://cocoadocs.org/docsets/GRMustache/6.7.4/Classes/GRMustacheConfiguration.html#//api/name/AVAILABLE_GRMUSTACHE_VERSION_6_4_AND_LATER):

@property (nonatomic, retain) GRMustacheContext *AVAILABLE_GRMUSTACHE_VERSION_6_4_AND_LATER

The property has, unfortunately, lost its name (and the macro should not appear in the doc).

http://github.com/groue/GRMustache actually uses those macros to decorate:

I use them to be able to keep alive the tests for deprecated but not-yet removed methods, without getting any warning - they are quite useful, actually.

Thanks for the good work 😄 (BTW the latest GRMustache is really faster than the one you are using, you may consider upgrading)

@paulmelnikow
Copy link
Contributor

Re your BTW: Do you know if the latest version of GRMustache works with garbage collection? If not I think the project will need to be ARC-ified first.

@groue
Copy link
Contributor Author

groue commented Jul 16, 2013

GRMustache is explicitly tested under GC, ARC-compatible, and does not require to be ARC-ified to integrate into any project.

I've looked at appledoc sources. Here are the old APIs that you would need to migrate:

  • GBDictionaryTemplateLoader is now built-in (you can drop this class entirely, and use +[GRMustacheTemplateRepository templateRepositoryWithDictionary:])
  • -[GBDictionaryTemplateLoader parseString:error:] is replaced by -[GRMustacheTemplateRepository templateFromString:error:]
  • -[GRMustacheTemplate renderObject:] is replaced by -[GRMustacheTemplate renderObject:error:]
  • syntax-wise, the slash in {{ foo/bar }} tags has been replaced by the dot, as in {{ foo.bar }}

@tomaz
Copy link
Owner

tomaz commented Jul 28, 2013

Is this still an issue with latest code? I merged pull request addressing some of output formatting problems.

@groue
Copy link
Contributor Author

groue commented Jul 30, 2013

Yes, the issue is still here, using appledoc 2.1 (build 858), installed from the latest master branch (ba1083e) and sh install-appledoc.sh -t default command.

@groue
Copy link
Contributor Author

groue commented Jul 30, 2013

The newly introduced support for enums are affected, too.

The following code:

/**
 * The codes of a GRMustache-generated NSError
 *
 * @since v1.0
 */
typedef NS_ENUM(NSInteger, GRMustacheErrorCode) {
    /**
     * The error code for parse errors.
     *
     * @since v1.0
     */
    GRMustacheErrorCodeParseError AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,

    /**
     * The error code for not found templates and partials.
     *
     * @since v1.0
     */
    GRMustacheErrorCodeTemplateNotFound AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,

    /**
     * The error code for not rendering errors.
     *
     * @since v6.3
     */
    GRMustacheErrorCodeRenderingError AVAILABLE_GRMUSTACHE_VERSION_6_3_AND_LATER,

} AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER;

Is documented as (6 values in the enum instead of 3, and mangled doc strings):

Definition

typedef NS_ENUM(NSInteger, GRMustacheErrorCode ) {
   GRMustacheErrorCodeParseError,
   AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,
   GRMustacheErrorCodeTemplateNotFound,
   AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,
   GRMustacheErrorCodeRenderingError,
   AVAILABLE_GRMUSTACHE_VERSION_6_3_AND_LATER,
};

Constants

  • GRMustacheErrorCodeParseError

    The error code for parse errors.
    Available in v1.0
    Declared In GRMustacheError.h.
    
  • AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER

    The codes of a GRMustache-generated NSError
    Available in v1.0
    Declared In GRMustacheError.h.
    
  • GRMustacheErrorCodeTemplateNotFound

    The error code for not found templates and partials.
    Available in v1.0
    Declared In GRMustacheError.h.
    
  • AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER

    The codes of a GRMustache-generated NSError
    Available in v1.0
    Declared In GRMustacheError.h.
    
  • GRMustacheErrorCodeRenderingError

    The error code for not rendering errors.
    Available in v6.3
    Declared In GRMustacheError.h.
    
  • AVAILABLE_GRMUSTACHE_VERSION_6_3_AND_LATER

    The codes of a GRMustache-generated NSError
    Available in v1.0
    Declared In GRMustacheError.h.
    

Availability

v1.0

@groue
Copy link
Contributor Author

groue commented Jul 30, 2013

Note that enums with values are better handled:

/**
 * The types of Mustache tags
 *
 * @since v6.0
 */
typedef NS_ENUM(NSUInteger, GRMustacheTagType) {
    /**
     * The type for variable tags such as {{ name }}
     *
     * @since v6.0
     */
    GRMustacheTagTypeVariable = 1 << 1 AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,

    /**
     * The type for section tags such as {{# name }}...{{/}}
     *
     * @since v6.0
     */
    GRMustacheTagTypeSection = 1 << 2 AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,

    /**
     * The type for overridable section tags such as {{$ name }}...{{/}}
     *
     * @since v6.0
     */
    GRMustacheTagTypeOverridableSection = 1 << 3 AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,

    /**
     * The type for inverted section tags such as {{^ name }}...{{/}}
     *
     * @since v6.0
     */
    GRMustacheTagTypeInvertedSection = 1 << 4 AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,
} AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER;

Is documented with 4 values, not 8, unlike the GRMustacheErrorCode enum above. Macros are still visible in the documentation, though.

@tomaz
Copy link
Owner

tomaz commented Jul 30, 2013

Yea, parsing is not exact in appledoc, I was looking into using clang, but it felt too exact for such needs. Will take a look once I finish my current work.

@robvdveer
Copy link
Collaborator

I'll pick this one up, if you don't mind, since i'm the one responsible... (The issues with the macro mismatch)

@tomaz
Copy link
Owner

tomaz commented Jul 30, 2013

Sure, go ahead

@robvdveer
Copy link
Collaborator

Any suggestions on how to detect macros without having to parse all include/import statements? Can i safely assume all uppercase is a macro? What about parametrised macros? If this is the case, a simple check would do to 'eat away' the capiltalized tokens

@groue
Copy link
Contributor Author

groue commented Jul 30, 2013

I'd assume that all macros are stuff that can be ignored after the longest documentation item has been identified:

After MACRO; has been parsed, no method has been identified (missing colon between MACRO and ;), so the identified method is blah:foo::

- (void)blah:(int)a foo:(int)b MACRO;

After the last ) has been parsed, the name property has been totally identified, and everything else can be ignored:

@property (nonatomic,copy) int(^name)(int arg) __attribute__((deprecated));

After name1, value, and } have been parsed, the name1, name2 and foo enum values and enum has been identified, and everything else can be ignored:

enum foo {
    name1 MACRO,
    name2 = value MACRO,
} MACRO;

What do you think?

This technique would avoid dangerous assumptions like uppercase macros.

@robvdveer
Copy link
Collaborator

I'd assume that all macros are stuff that can be ignored after the longest documentation item has been identified:

This approach may fail horribly if the macro contains () like MAX(1,2). I thought of the uppercase because that's how us humans look at it. For example, how would your approach see the difference between this?

typedef enum MACRO {
}  name;

and

typedef enum name {
} MACRO;

(I know this is not valid, but it's just an example). Furthermore, the MACRO can appear literally anywhere. Somebody might use a macro to define a block notation instead of a typedef like so

@property (nonatomic) IBOUTLET_MACRO_FOR_MY_SPECIAL_NSINTEGER(name)

In which case the parser will be totally lost. I think we can safely skip documenting such horrid definitions.

Another thing is feasability. We could either come up with a bulletproof parser taking us 50 days to write and test, or simply filter out the CAPS elements.

@groue
Copy link
Contributor Author

groue commented Jul 30, 2013

Not an easy job, you're right.

@robvdveer
Copy link
Collaborator

Should the MACRO directives persist in the documentation? I am now able to parse the NS_ENUM declarations by filtering the all uppercase words. Sample:

#define AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER

typedef NS_ENUM(NSUInteger, GSEnumDemo)
{
    One,
    Two AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,
    Three = 3,
    Four AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER = 4,
    Five = 1 << 5 AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER,
    Six = AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER 1 << 6,
    Seven
} AVAILABLE_GRMUSTACHE_VERSION_6_0_AND_LATER;

This will result in the documented:


Definition
typedef NS_ENUM(NSUInteger, GSEnumDemo ) {
   One,
   Two,
   Three = 3,
   Four = 4,
   Five = 1 < < 5,
   Six = 1 < < 6,
   Seven,
};

I added a method to the parser called 'consumeMacro' perhaps i can reuse it for the method declarations (changes pushed to my fork)

@tomaz
Copy link
Owner

tomaz commented Jul 30, 2013

I think it's a valid assumption for most cases.

@robvdveer
Copy link
Collaborator

That last push #381 fixed the behavior for NS_ENUM that was just introduced, but I didn't dare touching the other parsing code.

@tomaz
Copy link
Owner

tomaz commented Aug 1, 2013

Cool, is this fixed then, can I close the issue?

@robvdveer
Copy link
Collaborator

No, the push only fixed macros for NS_ENUM, not for properties as in the original poster request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants