Skip to content

Commit

Permalink
Implemented better Xcode integration for warnings and errors. Closes t…
Browse files Browse the repository at this point in the history
…omaz#105.

By using special log format `--logformat xcode`, appledoc formats warnings and errors related to source files in a way that Xcode is able to catch them and display them in build results. Currently only warnings are using that format (there is no error log message related to source files).

*Implementation details:* Note that implementation required tweaking Cocoa Lumberjack source. Specifically, it required passing in original source code filename and line number to log message. Although logging macros pick up source file and line automatically through `__FILE__` and `__LINE__`, these belong to appledoc sources. But for Xcode warnings making sense, we need to write the file name and line of the context being parsed. To reuse existing macros, I added both values to DDMessage and additional class method to DDLog which needs to be called prior to logging. The method will simply store info to static vars. Then logging method will pick up stored values, write them to message and reset static vars. When emitting log messages, Xcode formatter will check if these values are set and emit specially formatted logs, otherwise it will revert to message only.

*Additional note:* Although I searched for all occurrences of `GBLogWarn` and `GBLogError` in the project and updated those that seemed relevant, some may have been left out - let me know about exact messages and I will check it. Also note that parsing code currently doesn't emit warnings, if something goes wrong, exceptions are raised, which doesn't log the file and line. Didn't take much time to go into this as these usually indicate parsing code needs to be refined.
  • Loading branch information
tomaz committed May 25, 2011
1 parent 29c8e19 commit a63d7b5
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 11 deletions.
7 changes: 7 additions & 0 deletions Common/GBLog.h
Expand Up @@ -73,6 +73,10 @@ void GBLogUpdateResult(NSInteger result);
#define GBLogDebug(frmt, ...) SYNC_LOG_OBJC_MAYBE(kGBLogLevel, LOG_FLAG_DEBUG, frmt, ##__VA_ARGS__)
#define GBLogIsEnabled(level) ((kGBLogLevel & level) > 0)

// Macros that store given file/line info. Mostly used for better Xcode integration!
#define GBLogXError(source,frmt,...) { [DDLog storeFilename:[source filename] line:[source lineNumber]]; GBLogError(frmt, ##__VA_ARGS__); }
#define GBLogXWarn(source,frmt,...) { [DDLog storeFilename:[source filename] line:[source lineNumber]]; GBLogWarn(frmt, ##__VA_ARGS__); }

// Helper macros for logging exceptions. Note that we don't use formatting here as it would make the output unreadable
// in higher level log formats. The information is already verbose enough!
#define GBLogExceptionLine(frmt,...) { ddprintf(frmt, ##__VA_ARGS__); ddprintf(@"\n"); }
Expand Down Expand Up @@ -150,3 +154,6 @@ void GBLogUpdateResult(NSInteger result);

@interface GBLogFormat3Formatter : NSObject <DDLogFormatter>
@end

@interface GBLogFormatXcodeFormatter : NSObject <DDLogFormatter>
@end
23 changes: 23 additions & 0 deletions Common/GBLog.m
Expand Up @@ -54,6 +54,9 @@ + (void)setLogLevelFromVerbose:(NSString *)verbosity {
}

+ (id<DDLogFormatter>)logFormatterForLogFormat:(NSString *)level {
level = [level lowercaseString];
if ([level isEqualToString:@"xcode"]) return [[[GBLogFormatXcodeFormatter alloc] init] autorelease];

NSInteger value = [level integerValue];
if (value < 0) value = 0;
if (value > 3) value = 3;
Expand Down Expand Up @@ -154,3 +157,23 @@ - (NSString *)formatLogMessage:(DDLogMessage *)m {
return [NSString stringWithFormat:@"%@ | %@ ln %i > %@", GBLogLevel(m), GBLogFile(m), GBLogLine(m), GBLogMessage(m)];
}
@end

@implementation GBLogFormatXcodeFormatter
- (NSString *)formatLogMessage:(DDLogMessage *)m {
if (m->originalFilename) {
NSString *level = nil;
switch (m->logFlag) {
case LOG_FLAG_FATAL: level = @"fatal";
case LOG_FLAG_ERROR: level = @"error";
case LOG_FLAG_WARN: level = @"warning";
case LOG_FLAG_NORMAL: level = @"normal";
case LOG_FLAG_INFO: level = @"info";
case LOG_FLAG_VERBOSE: level = @"verbose";
case LOG_FLAG_DEBUG: level = @"debug";
default: level = @"unknown";
}
return [NSString stringWithFormat:@"%@:%u: %@: %@", m->originalFilename, m->originalLine, level, GBLogMessage(m)];
}
return GBLogMessage(m);
}
@end
9 changes: 9 additions & 0 deletions Common/ThirdParty/CocoaLumberjack/DDLog.h
Expand Up @@ -267,6 +267,12 @@ NSString *ExtractFileNameWithoutExtension(const char *filePath, BOOL copy);
line:(int)line
format:(NSString *)format, ...;

/** Sets the given filename and line to be stored to the next log message.
This is used for nicer Xcode integration when issuing warnings and errors: we don't want Xcode pointing to appledoc files, but instead to user's source files that caused warnings/errors! After next log:level:flag:file:function:line:format: call, these values are reset.
*/
+ (void)storeFilename:(NSString *)file line:(NSUInteger)line;

/**
* Since logging can be asynchronous, there may be times when you want to flush the logs.
* The framework invokes this automatically when the application quits.
Expand Down Expand Up @@ -440,6 +446,9 @@ NSString *ExtractFileNameWithoutExtension(const char *filePath, BOOL copy);
const char *function;
int lineNumber;
mach_port_t machThreadID;

NSString *originalFilename;
NSUInteger originalLine;

// The private variables below are only calculated if needed.
// You should use the public methods to access this information.
Expand Down
13 changes: 13 additions & 0 deletions Common/ThirdParty/CocoaLumberjack/DDLog.m
Expand Up @@ -468,6 +468,13 @@ + (void)queueLogMessage:(DDLogMessage *)logMessage synchronously:(BOOL)flag
}
}

static NSString *GBStoreFilename = nil;
static NSUInteger GBStoreLine = 0;
+ (void)storeFilename:(NSString *)file line:(NSUInteger)line {
GBStoreFilename = [file retain];
GBStoreLine = line;
}

+ (void)log:(BOOL)synchronous
level:(int)level
flag:(int)flag
Expand All @@ -489,6 +496,12 @@ + (void)log:(BOOL)synchronous
function:function
line:line];

if (GBStoreFilename) {
logMessage->originalFilename = GBStoreFilename;
logMessage->originalLine = GBStoreLine;
GBStoreFilename = nil;
}

[self queueLogMessage:logMessage synchronously:synchronous];

[logMessage release];
Expand Down
2 changes: 1 addition & 1 deletion Model/GBClassData.m
Expand Up @@ -44,7 +44,7 @@ - (void)mergeDataFromObject:(id)source {
if (![self nameOfSuperclass]) {
self.nameOfSuperclass = sourceClass.nameOfSuperclass;
} else if (sourceClass.nameOfSuperclass && ![self.nameOfSuperclass isEqualToString:sourceClass.nameOfSuperclass]) {
GBLogWarn(@"%@: Merged class's %@ superclass is different from current!", self, sourceClass);
GBLogXWarn(self.prefferedSourceInfo, @"%@: Merged class's %@ superclass is different from current!", self, sourceClass);
}

// Forward merging request to components.
Expand Down
2 changes: 1 addition & 1 deletion Model/GBModelBase.m
Expand Up @@ -47,7 +47,7 @@ - (void)mergeDataFromObject:(id)source {
// Merge comment.
GBComment *comment = [(GBModelBase *)source comment];
if (self.comment && comment) {
GBLogWarn(@"%@: Comment found in %@ and %@!", self, self.comment.sourceInfo, comment.sourceInfo);
GBLogXWarn(self.prefferedSourceInfo, @"%@: Comment found in %@ and %@!", self, self.comment.sourceInfo, comment.sourceInfo);
return;
}
if (!self.comment && comment) self.comment = comment;
Expand Down
10 changes: 5 additions & 5 deletions Processing/GBCommentsProcessor.m
Expand Up @@ -196,7 +196,7 @@ - (void)processCommentBlockInLines:(NSArray *)lines blockRange:(NSRange)blockRan
if ([self processExceptionBlockInString:string lines:lines blockRange:blockRange shortRange:shortRange]) return;
if ([self processReturnBlockInString:string lines:lines blockRange:blockRange shortRange:shortRange]) return;
if ([self processRelatedBlockInString:string lines:lines blockRange:blockRange shortRange:shortRange]) return;
GBLogWarn(@"Unknown directive block %@ encountered at %@, processing as standard text!", [[lines firstObject] normalizedDescription], self.currentSourceInfo);
GBLogXWarn(self.currentSourceInfo, @"Unknown directive block %@ encountered at %@, processing as standard text!", [[lines firstObject] normalizedDescription], self.currentSourceInfo);
}

// Handle short description and update block range if we're not repeating first paragraph.
Expand Down Expand Up @@ -361,7 +361,7 @@ - (BOOL)processRelatedBlockInString:(NSString *)string lines:(NSArray *)lines bl
// Convert to markdown and register everything. We use strict links mode. If the link is note recognized, warn and exit.
NSString *markdown = [self stringByConvertingCrossReferencesInString:reference withFlags:GBProcessingFlagRelatedItem];
if ([markdown isEqualToString:reference]) {
GBLogWarn(@"Unknown cross reference %@ found at %@!", reference, self.currentSourceInfo);
GBLogXWarn(self.currentSourceInfo, @"Unknown cross reference %@ found at %@!", reference, self.currentSourceInfo);
return YES;
}

Expand Down Expand Up @@ -449,7 +449,7 @@ - (NSString *)stringByPreprocessingString:(NSString *)string withFlags:(GBProces
markdownStartMarker = componentMarker;
}
else if (self.settings.warnOnUnknownDirective) {
GBLogWarn(@"Unknown format marker %@ detected at %@!", componentMarker, self.currentSourceInfo);
GBLogXWarn(self.currentSourceInfo, @"Unknown format marker %@ detected at %@!", componentMarker, self.currentSourceInfo);
}
if (!markdownEndMarker) markdownEndMarker = markdownStartMarker;

Expand Down Expand Up @@ -756,7 +756,7 @@ - (GBCrossRefData)dataForRemoteMemberLinkInString:(NSString *)string searchRange
if (!referencedObject) {
referencedObject = [self.store protocolWithName:objectName];
if (!referencedObject) {
if (self.settings.warnOnInvalidCrossReference) GBLogWarn(@"Invalid %@ reference found near %@, unknown object!", linkText, self.currentSourceInfo);
if (self.settings.warnOnInvalidCrossReference) GBLogXWarn(self.currentSourceInfo, @"Invalid %@ reference found near %@, unknown object!", linkText, self.currentSourceInfo);
result.range = [string rangeOfString:linkText options:0 range:searchRange];
result.markdown = [NSString stringWithFormat:@"[%@ %@]", objectName, selector];
return result;
Expand All @@ -767,7 +767,7 @@ - (GBCrossRefData)dataForRemoteMemberLinkInString:(NSString *)string searchRange
// Ok, so we've found a reference to an object, now search for the member. If not found, warn and return. Note that we mark the result so that we won't be searching the range for other links.
id referencedMember = [[referencedObject methods] methodBySelector:selector];
if (!referencedMember) {
if (self.settings.warnOnInvalidCrossReference) GBLogWarn(@"Invalid %@ reference found near %@, unknown method!", linkText, self.currentSourceInfo);
if (self.settings.warnOnInvalidCrossReference) GBLogXWarn(self.currentSourceInfo, @"Invalid %@ reference found near %@, unknown method!", linkText, self.currentSourceInfo);
result.range = [string rangeOfString:linkText options:0 range:searchRange];
result.markdown = [NSString stringWithFormat:@"[%@ %@]", objectName, selector];
return result;
Expand Down
8 changes: 4 additions & 4 deletions Processing/GBProcessor.m
Expand Up @@ -197,7 +197,7 @@ - (void)processParametersFromComment:(GBComment *)comment matchingMethod:(GBMeth
[names enumerateObjectsUsingBlock:^(NSString *name, NSUInteger idx, BOOL *stop) {
GBCommentArgument *parameter = [parameters objectForKey:name];
if (!parameter) {
GBLogWarn(@"%@: Description for parameter '%@' missing for %@!", comment.sourceInfo, name, method);
GBLogXWarn(comment.sourceInfo, @"%@: Description for parameter '%@' missing for %@!", comment.sourceInfo, name, method);
return;
}
[sorted addObject:parameter];
Expand All @@ -210,7 +210,7 @@ - (void)processParametersFromComment:(GBComment *)comment matchingMethod:(GBMeth
[description appendString:parameter.argumentName];
[sorted addObject:parameter];
}];
if (self.settings.warnOnMissingMethodArgument) GBLogWarn(@"%@: %ld unknown parameter descriptions (%@) found for %@", comment.sourceInfo, [parameters count], description, method);
if (self.settings.warnOnMissingMethodArgument) GBLogXWarn(comment.sourceInfo, @"%@: %ld unknown parameter descriptions (%@) found for %@", comment.sourceInfo, [parameters count], description, method);
}

// Finaly re-register parameters to the comment if necessary (no need if there's only one parameter).
Expand Down Expand Up @@ -385,12 +385,12 @@ - (void)mergeKnownCategoriesFromStore {

- (void)validateCommentsForObjectAndMembers:(GBModelBase *)object {
// Checks if the object is commented and warns if not. This validates given object and all it's members comments! The reason for doing it together is due to the fact that we first process all members and then handle the object. At that point we can even remove the object if not documented. So we can't validate members before as we don't know whether they will be deleted together with their parent object too...
if (![self isCommentValid:object.comment] && self.settings.warnOnUndocumentedObject) GBLogWarn(@"%@ is not documented!", object);
if (![self isCommentValid:object.comment] && self.settings.warnOnUndocumentedObject) GBLogXWarn(object.prefferedSourceInfo, @"%@ is not documented!", object);

// Handle methods.
for (GBMethodData *method in [[(id<GBObjectDataProviding>)object methods] methods]) {
if (![self isCommentValid:method.comment] && self.settings.warnOnUndocumentedMember) {
GBLogWarn(@"%@ is not documented!", method);
GBLogXWarn(method.prefferedSourceInfo, @"%@ is not documented!", method);
}
}
}
Expand Down

0 comments on commit a63d7b5

Please sign in to comment.